Opened on 10/17/2016 at 11:58:12 AM
Closed on 11/03/2016 at 10:15:18 AM
#4540 closed change (fixed)
Update createNightlies script to support creating dev builds from bookmarked branches
Reported by: | kzar | Assignee: | jsonesen |
---|---|---|---|
Priority: | P2 | Milestone: | |
Module: | Sitescripts | Keywords: | |
Cc: | trev, kvas, sebastian, oleksandr | Blocked By: | |
Blocking: | #4551 | Platform: | Unknown / Cross platform |
Ready: | yes | Confidential: | no |
Tester: | Unknown | Verified working: | no |
Review URL(s): |
Description (last modified by kzar)
Background
We are moving Safari support for adblockpluschrome into a separate safari bookmark. That will allow us to remove Safari specific code, workarounds and hacks from the main code. We plan to then make the minimum required changes to the code under the safari bookmark to keep things working.
We also plan to add Edge support for adblockpluschrome, until it is ready to be merged into the master bookmark we want to develop that under a separate edge bookmark too.
For both the safari and edge bookmarks we need to be able to control the builds properly:
Changes to the safari or edge branches should only trigger a development build for the relevant platform. Changes to the master branch should only trigger a development build for the other supported platforms.
What to change
Add a configuration option to set the branch to be used for development builds, e.g. setting abpsafari_bookmark = safari should make sitescripts.extensions.bin.createNightlies check out the safari bookmark. If no such configuration option is given, the default should be master (as opposed as default that we use right now).
Attachments (0)
Change History (17)
comment:1 Changed on 10/17/2016 at 12:05:32 PM by trev
- Component changed from Build-and-Release-Tools to Sitescripts
- Description modified (diff)
- Summary changed from Update buildtools to support boomarks for dev and release builds to Update createNightlies script to support creating dev builds from bookmarked branches
comment:2 Changed on 10/17/2016 at 01:16:12 PM by kzar
comment:3 Changed on 10/19/2016 at 08:52:12 AM by sebastian
- Blocking 4028 removed
- Ready set
I don't see why #4028 depends on this issue. Although for Microsoft Edge we might need both for now, implementing the packager and creating devbuilds out of a bookmark, can be tackled separately.
comment:4 Changed on 10/19/2016 at 06:51:51 PM by kzar
- Blocking 4551 added
comment:5 Changed on 10/19/2016 at 07:55:08 PM by jsonesen
Hi,
I will take this ticket and prioritize it for getting into review today. One question I have is regarding the nightlies config file. Can I have an example of that to use for testing? I want to add tests for this change.
comment:6 Changed on 10/19/2016 at 08:12:30 PM by kzar
Awesome, thanks Jon. Unless I'm missing something this will use the global sitescripts config. If so there's an example in the sitescripts repository.
comment:7 Changed on 10/19/2016 at 09:03:37 PM by kzar
Ah I think I understand now, you mean the nightliesData=%(root)s/data/nightlies file? If so I'm guessing Wladimir would be a good person to ask for an example of that, or someone else with server access.
comment:8 Changed on 10/19/2016 at 10:09:38 PM by jsonesen
- Owner set to jsonesen
comment:9 Changed on 10/19/2016 at 10:19:16 PM by jsonesen
The config appears to be generated at the time of the nightly creation so i'lll be okay without it. My plan for the implementation is as follows:
Add attribute to the Nightlybuild objects init function which will look for the presence of a config parameter in the extensions section of the config that starts with the platform type (in the code it is self.config.type) and ends with 'branch' for example: safari_branch = safari if the conf item is found it will do self.branch = self.config.get('extensions', self.config.type+'_branch') or something similar then it will use that as the branch in all mercurial calls throughout the methods that call it.
In the description you preface the platform with abp which I can also do, is that preferable?
comment:10 Changed on 10/19/2016 at 10:39:57 PM by jsonesen
for now I will add the abp prefix
comment:11 Changed on 10/20/2016 at 12:15:58 PM by kzar
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:12 Changed on 10/21/2016 at 10:05:19 AM by kzar
- Description modified (diff)
comment:13 follow-up: ↓ 14 Changed on 10/25/2016 at 04:21:52 PM by trev
@kzar: Why did you rename it from abpsafari_branch to abpsafari_bookmark? Bookmarks are merely one branching mechanism, why should we hardcode implementation details here? In fact, this should work fine with any revision identifier so I would be fine with abpsafari_revision as well.
comment:14 in reply to: ↑ 13 Changed on 10/25/2016 at 06:44:13 PM by kzar
Replying to trev:
@kzar: Why did you rename it from abpsafari_branch to abpsafari_bookmark? Bookmarks are merely one branching mechanism, why should we hardcode implementation details here? In fact, this should work fine with any revision identifier so I would be fine with abpsafari_revision as well.
Well because I thought it was clearer, branches in Mercurial are something else. But sure you're right as well and I don't insist.
comment:15 Changed on 10/26/2016 at 03:47:39 PM by jsonesen
Ill just call it revision then
comment:16 Changed on 11/02/2016 at 11:10:26 AM by abpbot
A commit referencing this issue has landed:
Issue 4540 - Add Platform Specific Branch Support to createNightlies.py
comment:17 Changed on 11/03/2016 at 10:15:18 AM by kzar
- Resolution set to fixed
- Status changed from reviewing to closed
Vasily any chance you could prioritise this issue? Once it's done we can set up the safari bookmark and begin stripping out a whole bunch of code from master. I'm hoping the next adblockpluschrome release will be the last one to include Safari.