adblockplusui builds on Windows are broken

[oleksandr]

Environment

Windows 10, Fall Creators Update

How to reproduce

1. clone adblockplusui
2. run npm install
3. observe an error:

npm ERR! Failed at the adblockplusui@ bash:js script 'eslint ./js/**/*.js && bash -c 'echo "/* eslint-disable */$(browserify --node --no-bundle-external $0)">$1' "./js/desktop-options.js" "./desktop-options.js"'.

Expected behaviour

There is no bash on Windows, so we should replace that with cmd.

[sebastian]

Alternatively, we could write a JS script which then can run in any environment.

[agiammarchi]

This is a very valid point. I have assumed Windows builds would've run under Linux, macOS, WSL Bash, or git-shell for Windows console, which is a quite standard environment and most of the bash in there "just works".

If that is not the case, and builds for Windows always run on regular windows, I agree with @sebastian we need a JS script that would work in there too, which is preferable than cmd and more appropriate for a JSON file (no need to detect and branch inline).

[greiner]

That sounds very much like #6303 (part of #6117) which is still open. There we mention using "gulp" for orchestration.

@Andrea Is #6303 supposed to be closed since a commit has already landed or do you prefer we keep it open for this?

[agiammarchi]

@Thomas I forgot to close that, which I've done now. That was about basics, which are in. Now basics need to be improved. I'd like to have my current modules in review out before though, otherwise will be a mess if we change things with many reviews based on that approach already.

I also kinda think if we have to move to an orchestrator then we might be better off hooking ourselves into parent folder Webpack (or change the build tools to bundle us as well)

Having Gulp in a subfolder handled by Webpack seems a bit odd to me.

[tlucas]

FYI - Brent (getadblock.com) just ran into this issue, preventing proper development on Windows.

[agiammarchi]

@tlucas can Brent generate builds for Windows using same stack we do? (Linux / macOS) AFAIK there is not much of a priority to change this in the short term so I'm trying to understand if we should, instead, prioritize (considering the imminent code freeze too).

Thanks in advance for expanding about this.

[sebastian]

As far as I can tell, its not a super blocker. Building Adblock Plus with ​Bash on Ubuntu on Windows does as a workaround, and even if we get a fix in Adblock Plus 3.2, AdBlock will likely update the code base anyway before Adblock Plus 3.2 is released (using one workaround or another).

However, if this issue is all that is preventing Adblock Plus from being built successfully on native Windows, than we should just address it. Also our own developers who use Windows would much prefer to be able to build Adblock Plus in a native environment, not to mention confused contributors.

[agiammarchi]

This is surely something that needs to be addressed at some point, but since Windows has various options for developers (WSL, Git shell, MinGW, others) and all of them would build natively for Windows, we'd like to not rush any workaround (and rather take time to properly implement a Webpack configuration in a way that is future-compatible with our parent folder).

[oleksandr]

Is there an ETA for when this could be fixed?

[agiammarchi]

I think the main issue here is that cmd is not a bash substitute but Windows has many ways to run bash, including WLS, Git for Windows, others such chocolatey where you can simply choco install git to have usable bash everywhere.

Since Windows developers are rare but also capable of installing bash, which is usually always available since it's the most used nodejs tool for scripts, why should we fix this or how much important this really is?

[oleksandr]

Currently this is the only issue that prevents us to build ABP for Microsoft Edge on Windows using native Windows tools. This is rather unfortunate, and we have made great effort in past to ensure Windows developers aren't treated as second class citizens.

Right now we can get by with WSL and such, but even using ​ManifoldJS for producing .appx package works differently on Windows and Linux. I think it is only a matter of time when this will bite us. Frankly, I don't see enough reasons for effectively dropping support of builds on Windows.

And Sebastian's point about third-party contributors is valid too.

[agiammarchi]

Frankly, I don't see enough reasons for effectively dropping support of builds on Windows.

I don't think it's our plan to officially drop support, and it would just take a lot of time to change current settings so it's nowhere in our schedule because none of us uses Windows, no external contributor ever complained (so far) but also everything works via WSL and/or bash for git and others solutions.

I guess my answer, AFAIK, is simply that there is no ETA for now, and we are shipping soon tons of changes with a lot of high priority tickets, so I don't know when we'll have time to change our build tools, sorry.

Maybe @greiner has more ideas when this might happen.

[agiammarchi]

P.S. just thinking that having NodeJS is a requirement to build ABP so what if we simply make Bash a requirement too, providing links / solutions in the ABP UI readme? Would that be a better workaround for now?

[oleksandr]

no external contributor ever complained

Well, from the comments above Brent apparently complained. Inside the company, I use Windows, and I am not sure what Georgiana is using, but it's not a question of what we use, but of what we support. I agree it shouldn't top your priorities, of course, just trying to put this somewhere on the roadmap.

simply make Bash a requirement too, providing links / solutions in the ABP UI readme

Yes, it's ok for now, but instructions would also need to be added in adblockpluschrome. FWIW just running npm install in Bash once after checkout within adblockplusui directory is enough to keep working with native tools.

[agiammarchi]

by any chance you have experience with this NodeJS library ?
​https://github.com/shelljs/shx

if yes, do you think that might be a solution?

[oleksandr]

Never used it myself, but looks like exactly what we need here!

[BrentM]

Our team definitely does development work on Windows 10 (and mac os), so a long term solution is desirable.

I glanced at the ShellJS and Shx (I haven't used them either), but it seems to be the missing library that's needed.

[agiammarchi]

Update

I gave ShellJS Shx a chance, but it's not as powerful as I hoped. Most basic needed parts such passed arguments are ignored so that everything throws "like a charm".

However, since Git for Windows automatically install, by default, a usable bash executable, I wonder if the following could be the solution:

npm config set script-shell "C:\\Program Files (x86)\\git\\bin\\bash.exe"

Being sure that it's either C:\\Program Files (x86)\\git\\bin\\bash.exe or C:\\Program Files\\git\\bin\\bash.exe.

That would align every script based setup (tons of them in GitHub) to use Bash instead of cmd as suggested already long long time ago in SO ​https://stackoverflow.com/questions/23243353/how-to-set-shell-for-npm-run-scripts-in-windows

TL;DR it looks like Windows developers already have bash, they just don't know/use it yet in conjunction with npm. Could this be a solution?

[greiner]

Considering the last comment, is this ticket still relevant or can we close it? Because unless the requirement to support Windows becomes explicit there will be further deviations from compatibility with Windows which will make it harder to establish support.

[sebastian]

I would still love to see native Windows support. Currently, in our ​README we instruct developers on Windows to set up a WSL environment, install the build dependencies in there, and run build.py (which in turn calls npm install in adblockplusui) from within Bash. However, npm test (in order to automate browsers installed on Windows) still needs to be executed from the native Windows environment (i.e. cmd.exe or PowerShell), where therefore Node.js (and our npm dependencies) need to be available as well. All of this just so that you can use UNIX shell script syntax in your npm scripts.

[greiner]

Ok, in that case it seems that we'd need to

1. Translate commands in package.json to JavaScript
2. Look into whether it'd be easier/worthwhile keeping "npm-dollar" around or replacing it with a Node-based build system
3. Adapt existing Node scripts to make them OS-agnostic

For that, we could prioritize use cases that are relevant to anyone outside the UI team (i.e. postinstall).

Does that sound about right?

[agiammarchi]

FWIW, we execute npm test in our CI and it has nothing to do with browsers so I am not sure I am understanding the issue, 'cause it looks not related to UI.

Bash in package.json is a super common language and there are tons of ways to work around this, let's please at lest be sure changes are needed on our side, since WSL is (IMHO) the way to go and every Windows dev would, at some point, be there, which is why it's awesome, not something to avoid (still IMHO).

Just to clarify, I'm not pushing back changes, I'm just saying we, UI, have used with ease bash and we are the one that work on UI the most so changing everything would not bring us much.

[agiammarchi]

P.S. I've written how to run UI in WSL and if you setup a command in .bashrc the Worg bit can be automated too.
​https://medium.com/@WebReflection/testing-safari-via-bash-for-windows-js-2b7b9098d9c0

[greiner]

Closing this ticket in favor of ​ui#574.