[Move core logic into adblockpluscore repository] Move core logic

[kzar]

Background

We need to move the core extension logic from the adblockplus repository to the adblockpluscore repository before we can start using it as a the core dependency from all the extension repositories.

Felix has performed this experimentally successfully, now we need to do it for real.

What to change

Move all core logic from the adblockplus repository into the adblockpluscore repository.

Felix please add your notes of the steps required here?

[fhd]

Sorry for the delay, here's what I did back in January to extract the core code into the adblockpluscore repository. I have a very strong opinion that we need to preserve the history, here's how I did that:

1. Create a filemap for all the files in ​the Core module, basically a file that defines which paths to include (and optionally what the new path should be, but it doesn't seem we need that, see below).
2. hg convert adblockplus adblockplus-temp --filemap filemap
3. cd adblockpluscore && hg pull -f ../adblockplus-temp
4. hg merge && hg commit

I've done this, additionally throwing away the tags (I don't think those are particularly useful in adblockpluscore), and I ended up with an adblockpluscore repository that has all the core code under lib/, along with all the history. hg log --graph shows what actually happenend: The core code was being developed in a different branch than typed objects until the merge.

I've put the result of this up on BitBucket: ​https://bitbucket.org/fhd/adblockpluscore.

But since I'll remove that at some point, here's the repository structure I ended up with:

├── jshydra
│   ├── ...
├── lib
│   ├── antiadblockInit.js
│   ├── downloader.js
│   ├── elemHide.js
│   ├── filterClasses.js
│   ├── filterListener.js
│   ├── filterNotifier.js
│   ├── filterStorage.js
│   ├── matcher.js
│   ├── notification.js
│   ├── subscriptionClasses.js
│   ├── synchronizer.js
│   ├── typedObjects
│   │   ├── arrayTypes.js
│   │   ├── objectTypes.js
│   │   ├── primitiveTypes.js
│   │   ├── references.js
│   │   ├── stringType.js
│   │   └── utils.js
│   └── typedObjects.js
├── run_tests.py
└── test
    ├── common.js
    ├── index.html
    ├── jquery-1.7.1.min.js
    ├── qunit.css
    ├── qunit.js
    └── tests
        └── typedObjects.js

That makes sense to me when it comes to the general structure. What's not ideal is that the tests are really only for typed objects, not the core code. But since the core code needs Chrome privileges, we can't run the tests outside a Firefox extension, i.e. adblockplustests. I suppose for now we should just document this, and think about how we can migrate the core tests later. We could, however, consider moving everything related to typedObjects (including tests) to a separate directory, no strong opinion though.

[fhd]

I guess the only thing we really need to do is to agree on when to perform this migration. We should update adblockplus right after moving the core code out, so we don't end up with duplicate code.

[fhd]

Now is the time to do this.

Note that I am _not_ planning to filter the adblockplus repository. I don't want to mess with the history and invalidate old refs. Instead, I am just going to hg rm the files that I moved (including their history) to adblockpluscore.

[fhd]

I'm running into some complications because of the e10s and 2.6.12 branches that are making their way from the adblockplus to the adblockpluscore repository. Both have been merged multiple times, so making them disappear for good with hg graft is non-trivial. In addition, I believe we would have created those branches in adblockpluscore all the same, if the code in question had been there already back when those branches were created.

So I would just like to rename them, prepending firefox or something to that effect. Asked Wladimir on IRC what he thinks, let's see.

[fhd]

Wladimir was in favour of getting rid of the branches, fair enough.

The easiest (well, still not trivial :D) way of doing that appears to be to add a branchmap and a splicemap to hg convert.

So here's what I did, I ran hg convert with the following filemap (one more file than currently listed on adblockplus.org/modules):

include "lib/antiadblockInit.js"
include "lib/cssRules.js"
include "lib/downloader.js"
include "lib/elemHide.js"
include "lib/filterClasses.js"
include "lib/filterListener.js"
include "lib/filterNotifier.js"
include "lib/filterStorage.js"
include "lib/matcher.js"
include "lib/notification.js"
include "lib/subscriptionClasses.js"
include "lib/synchronizer.js"

And the following branchmap:

e10s default
2.6.12-release default

Aaand the following splicemap (had to resort to pen and paper to come up with that one, the history is a bit of a mess):

3d7a22037cca8a32583e7050aecd32e24a049702 e94f02978ff993c97595d9f96f46d2a2df7da199
837fa42d0118d2a98fdc59c11163af71273e6a72 0542ba7f5a57a13a317ac1b8e44cc377757628bb
f484b17f0a54d372be4dc1b23d0181a38073910e d85bc935421077d44b4125b708b37dd532804a58

The resulting repository has a bunch of leaf nodes that I then stripped out manually (merge commits, branch close commits and duplicate commits). hg convert can probably do this somehow, but I didn't research it any more, it's fairly straightforward to do manually.

I've verified that all the commits that have to be there are there, and that all the files have the same content, and then pushed the results of this surgical adventure to ​https://bitbucket.org/fhd/adblockpluscore

[fhd]

And here goes an adblockplus repo that uses the core code from adblockpluscore: ​https://bitbucket.org/fhd/adblockplus

Waiting for feedback now. If the repos are fine, I can push them to hg.adblockplus.org.

[fhd]

Wladimir observed that I'm not extracting the history of the previous names of the relevant files. He's right, they were renamed 2-3 times each. I used hg log --follow and some minor trickery to get all the past names, and added them all to the filemap. Here's the result:

include "lib/antiadblockInit.js"
include "lib/cssRules.js"
include "lib/downloader.js"
include "lib/elemHide.js"
include "lib/filterClasses.js"
include "lib/filterListener.js"
include "lib/filterNotifier.js"
include "lib/filterStorage.js"
include "lib/matcher.js"
include "lib/notification.js"
include "lib/subscriptionClasses.js"
include "lib/synchronizer.js"

include "modules/ElemHide.jsm"
include "chrome/content/elemhide.js"
include "modules/FilterClasses.jsm"
include "chrome/content/filterClasses.js"
include "chrome/content/filterListener.js"
include "filterListener.js"
include "modules/FilterListener.jsm"
include "filterNotifier.js"
include "modules/FilterNotifier.jsm"
include "chrome/content/filterStorage.js "
include "filterStorage.js "
include "modules/FilterStorage.jsm"
include "chrome/content/matcher.js "
include "matcher.js "
include "modules/Matcher.jsm"
include "chrome/content/subscriptionClasses.js "
include "modules/SubscriptionClasses.jsm "
include "subscriptionClasses.js"
include "chrome/content/synchronizer.js "
include "modules/Synchronizer.jsm "
include "synchronizer.js "

Most of these files have existed since revision 845 in 2008, the others were added later. I'm not including any history from before revision 845 here - it's both tedious and not really useful. We're moving files here, so in my mind, extracting the history of those files is sufficient. Besides, revision 845 was pretty much a rewrite of that code, the history doesn't look useful. And it will still remain in the adblockplus repository, should that level of archaeology become necessary.

Unfortunately this adds more branches I'll have to make disappear. I'll work on an updated branchmap and splicemap...

[fhd]

Sebastian observed that chrome/content/cssProperties.js has to be added too.

[fhd]

Dave has observed that chrome/content/ui/subscriptions.xml has to be added too.

[fhd]

Alright, some progress. Here is the new filemap:

include "lib/antiadblockInit.js"
include "lib/cssRules.js"
include "lib/downloader.js"
include "lib/elemHide.js"
include "lib/filterClasses.js"
include "lib/filterListener.js"
include "lib/filterNotifier.js"
include "lib/filterStorage.js"
include "lib/matcher.js"
include "lib/notification.js"
include "lib/subscriptionClasses.js"
include "lib/synchronizer.js"
include "chrome/content/ui/subscriptions.xml"
include "chrome/content/cssProperties.js"

include "modules/ElemHide.jsm"
include "chrome/content/elemhide.js"
include "modules/FilterClasses.jsm"
include "chrome/content/filterClasses.js"
include "chrome/content/filterListener.js"
include "filterListener.js"
include "modules/FilterListener.jsm"
include "filterNotifier.js"
include "modules/FilterNotifier.jsm"
include "chrome/content/filterStorage.js "
include "filterStorage.js "
include "modules/FilterStorage.jsm"
include "chrome/content/matcher.js "
include "matcher.js "
include "modules/Matcher.jsm"
include "chrome/content/subscriptionClasses.js "
include "modules/SubscriptionClasses.jsm "
include "subscriptionClasses.js"
include "chrome/content/synchronizer.js "
include "modules/Synchronizer.jsm "
include "synchronizer.js "
include "chrome/content/subscriptions.rdf"
include "chrome/content/ui/subscriptions.rdf"

I realised that I only have to map the e10s branch into default explicitly, so here's the new branchmap:

e10s default

Since the new files were added, I had to recreate the splicemap:

e94f02978ff993c97595d9f96f46d2a2df7da199 56272888090ae81fd9e732c08f18d918917472c0
c0345d415b3248729043aa5933bfb565b170b908 0542ba7f5a57a13a317ac1b8e44cc377757628bb
837fa42d0118d2a98fdc59c11163af71273e6a72 e7f419bb37d493f6a2ffbac9639756468200d048
3403526566255ccb9b45fa2404500f97565c5387 1904a1d7276fc3999ebe6eedb437843e966990a3

I wrote some documentation for what I did there this time:

# Last in default before e10s merge
Issue 2395 - Added content script for CSS property filters
56272888090ae81fd9e732c08f18d918917472c0

# First in e10s
Issue 3208 - Separate contentPolicy module into a parent and child part
e94f02978ff993c97595d9f96f46d2a2df7da199

# Last in e10s
Issue 3251 - Delegate processing of element hiding hits to [...]
0542ba7f5a57a13a317ac1b8e44cc377757628bb

# First in default after e10s merge
Issue 2397 - Remove redundant load + apply calls
c0345d415b3248729043aa5933bfb565b170b908

# First in default after 2.6.12-release merge
Noissue - Updated documentation of ElemHideBase.fromText
837fa42d0118d2a98fdc59c11163af71273e6a72

# Last in default before 2.6.12-release merge
Issue 3222 - Expose filter type as a string property
e7f419bb37d493f6a2ffbac9639756468200d048

# First in default after ADBLOCK_PLUS_1_0_BRANCH merge
Improved source code documentation: made sure file descriptions are recognized and all classes have description
3403526566255ccb9b45fa2404500f97565c5387

# Last in default before ADBLOCK_PLUS_1_0_BRANCH merge
Made sure all initialization happens when abp.init() is called rather than at module load time
1904a1d7276fc3999ebe6eedb437843e966990a3

To retain even more sanity, I wrote some fairly hacky scripts so I wouldn't have to do any manual work anymore:

stripcopy first strips out some branches manually (could have done it with the splicemap, but I don't feel like redoing this):

#!/bin/bash
rm -rf adblockplus-stripped
cp -r adblockplus adblockplus-stripped
hg strip -R adblockplus-stripped 73a179e732e9
hg strip -R adblockplus-stripped cc4f0284674a
hg strip -R adblockplus-stripped a2d838950f9d

Then convert invokes hg convert and strips out the tags:

#!/bin/bash
rm -rf adblockplus-converted
hg convert --filemap filemap --branchmap branchmap --splicemap splicemap adblockplus-stripped adblockplus-converted
# Strip tags
hg strip -R adblockplus-converted tip

Finally, stripconverted strips out all the unmerged heads:

#!/bin/bash
while :
do
  heads=`hg log -R adblockplus-converted -r "head()-parents(merge())-tag(tip)" --template '{node}\n'`
  [[ -z $heads ]] && break
  for head in $heads
  do	      
    hg strip -R adblockplus-converted $head
  done
done

That's what I have so far. I've pushed my results to ​https://bitbucket.org/fhd/adblockpluscore.

There are only two problems left:

1. The experimental branch isn't stripped out yet. There has been lots of merging between that one and default however, that's going to be some tedious work I'm saving for next week.

2. Dave tried to use the new adblockpluscore repo in adblockpluschrome and noticed that the latter currently uses a couple of locale files from the adblockplus repository. I'm not really sure if they belong into adblockpluscore, however. Wladimir?

Also, for the record: Instead of merging this into the existing adblockpluscore, I've started a fresh repository, leaving out typed objects for now. We can later add those commits as a separate branch or fork, but I think the history from revision 0 on should be the "real" core history, that one shouldn't start on top of typed objects.

[fhd]

Also updated ​https://bitbucket.org/fhd/adblockplus now to work with the latest adblockpluscore.

[kzar]

Replying to fhd:

2. Dave tried to use the new adblockpluscore repo in adblockpluschrome and noticed that the latter currently uses a couple of locale files from the adblockplus repository. I'm not really sure if they belong into adblockpluscore, however. Wladimir?

I'm inclined to agree that those files probably don't belong in adblockpluscore.

Also, for the record: Instead of merging this into the existing adblockpluscore, I've started a fresh repository, leaving out typed objects for now. We can later add those commits as a separate branch or fork, but I think the history from revision 0 on should be the "real" core history, that one shouldn't start on top of typed objects.

Definitely agree with you there, especially as we're no longer definitely going to use typed objects!

[fhd]

Alright, let's leave those files out of Core then.

Also discussed the experimental branch with Wladimir: It requires quite some effort to cut it out cleanly, and frankly, it's not worth the effort. The experimental branch hasn't been used since 2011, and it was just part of the development work flow until then. We can just leave it alone.

That means that the repository in its current form is basically final. I've just pushed a commit that closes the experimental branch. Once we've confirmed that everything that needs to be in the repo is there, I can push it to hg.adblockplus.org.

[fhd]

There's one hacky script I didn't show yet: compare, I used that one to double check the results of the extraction:

#!/bin/bash

files="lib/antiadblockInit.js lib/cssRules.js lib/downloader.js lib/elemHide.js lib/filterClasses.js lib/filterListener.js lib/filterNotifier.js lib/filterStorage.js lib/matcher.js lib/notification.js lib/subscriptionClasses.js lib/synchronizer.js chrome/content/ui/subscriptions.xml chrome/content/cssProperties.js"

pushd adblockplus
hg update default
hg log --template '{firstline(desc)}\n' $files | sort > ../adblockplus-log
popd

pushd adblockplus-converted
hg update default
hg log --template '{firstline(desc)}\n' $files | sort > ../adblockplus-converted-log
popd

for file in $files
do
  diff -u "adblockplus/$file" "adblockplus-converted/$file"
done

diff -u adblockplus-log adblockplus-converted-log

Running this against the latest version of the repos yields the following result:

/foo/adblockplus /foo
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
/foo
/foo/adblockplus-converted /foo
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
/foo
--- adblockplus-log	2016-02-01 00:36:26.000000000 +0100
+++ adblockplus-converted-log	2016-02-01 00:36:26.000000000 +0100
@@ -7,9 +7,9 @@
 Added more parameters to the requests made by downloader
 Addressed review comments from http://codereview.adblockplus.org/11175021/
 Avoid creating getters in Filter.fromText, this is slow
-Backed out changeset 2d52467436ed (Issue 427 - Remove non-standard function and getter syntax) - dropping compatibility with Firefox 21 and below wasn't intentional
-Backed out changeset 69871d10cf2a
-Backout out revision 8ce1cfb4d7f6 (the original change was correct)
+Backed out changeset 74c38c6447d9 (Issue 427 - Remove non-standard function and getter syntax) - dropping compatibility with Firefox 21 and below wasn't intentional
+Backed out changeset e96a48589e44
+Backout out revision be805e221092 (the original change was correct)
 Do not initialize global stylesheet in Chrome
 Don't delay Firefox startup
 Don't use "for each" loop on a non-array, to avoid issues when the code is translated for Chrome
@@ -86,7 +86,6 @@
 Issue 656 - Replace some __proto__ with Object.create
 Made Notification module actually download notifications
 Mark filters with unknown options as invalid, don't attempt to use them
-Merged 1.0 branch to trunk (no longer required)
 Merged experimental branch, the new UI landed
 Moved initAntiAdblockNotification function from ui.js to its own file
 Moved modules to the lib/ directory
@@ -97,9 +96,9 @@
 Noissue - Updated documentation of ElemHideBase.fromText
 Notificatinos: implemented better target checks
 Notifications: "timestamp" field should be called "id", it doesn't have to be a timestamp
-Properly adjust code to changeset 34128d65fa18 - Filter.fromText() can no longer return null but it should not be passed empty filters
+Properly adjust code to changeset 20b6a8a7ba83 - Filter.fromText() can no longer return null but it should not be passed empty filters
 Refactor generic logic from Synchronizer into a Downloader module
-Relanded changeset 2d52467436ed (Issue 427 - Remove non-standard function and getter syntax) with appropriate compat info changes
+Relanded changeset 74c38c6447d9 (Issue 427 - Remove non-standard function and getter syntax) with appropriate compat info changes
 Relicensed from MPL to GPL
 Removed Do-Not-Track handling, $donottrack filter option is still being recognized to prevent issues
 Removed not very useful check for empty filters
@@ -115,8 +114,8 @@
 Subscription downloads: Remove undocumented support for passing version number in URL
 Subscription downloads: drop special handling for HTTP redirects
 Topic 10547 - Fixed: Automatic filter synchronization doesn't start up
-Topic 10992 - Fallback if patterns.ini doesn't exist doesn't work (regression from revision c9a77e4fdeff)
-Topic 10992 - Fallback if patterns.ini doesn't exist doesn't work (regression from revision c9a77e4fdeff)
+Topic 10992 - Fallback if patterns.ini doesn't exist doesn't work (regression from revision 072b62ab1a91)
+Topic 10992 - Fallback if patterns.ini doesn't exist doesn't work (regression from revision 072b62ab1a91)
 Topic 4402 - Redesigned "add filter subscription" dialog
 Topic 5579 - Introduce element hiding exceptions of the form domain#@#selector to allow authors of complementary subscriptions partially disable element hiding rules of the main subscription
 Topic 9548 - Fixed our behavior concerning trailing dots in domain names: ignore for blocking rules, keep for element hiding rules
@@ -208,7 +207,6 @@
 Updated list of recommended subscriptions
 Updated list of recommended subscriptions
 Updated list of recommended subscriptions
-Updated list of recommended subscriptions
 Updating default subscription suggestions - EasyList combinations should use HTTPS
 Updating homepage link for Dr.Evil
 Updating homepage link for Morpeh list

The files are identical, but the commit history differs a little bit. Largely this is because of changed changeset IDs. Some commits have disappeared because they were deliberately thrown out by the splice map - duplicate commits and merge commits.

I haven't noticed any issues.

[fhd]

Setting this to P2, since it currently blocks all work in ABP for Firefox.

[fhd]

I've pushed typed objects as an unrelated branch (marked with the typed-objects) bookmark to ​https://bitbucket.org/fhd/adblockpluscore now and asked Wladimir to have a final look before I push this.

[fhd]

One curious case deserves to be documented: I had to close the experimental branch, because ​the commit that closed it didn't make it through hg convert, presumably it has a child, ​a merge commit, which didn't touch any of the files in the filemap.

[fhd]

Aaand it's done:

​https://hg.adblockplus.org/adblockpluscore
​https://github.com/adblockplus/adblockpluscore