Scoping the tools

[ostrolucky]

I wanted to contribute scoping of one of the tools that toolbox ships because there are scoping issues when running it in one of our projects. To do this, I expected some tools would already be scoped so I can get inspiration what modifications in tools.json file I need to make. But looking into this json file, I don't see any such configuration in existing tools. Does this mean toolbox currently does not do scoping of any of the current tools whatsoever?

Ref deprecated-packages/symplify#1872

[jakzal]

I don't remember why ecs is currently installed with composer-install:

toolbox/resources/tools.json

Lines 116 to 127 in 93b5b64

	{
	"name": "ecs",
	"summary": "Sets up and runs coding standard checks",
	"website": "https://github.com/Symplify/EasyCodingStandard",
	"command": {
	"composer-install": {
	"repository": "https://github.com/Symplify/EasyCodingStandard.git",
	"target-dir": "%target-dir%/EasyCodingStandard"
	}
	},
	"test": "ecs -h"
	},

We could migrate it to composer-bin-plugin, where a namespace can be specified, like here:

toolbox/resources/tools.json

Lines 103 to 115 in 93b5b64

	{
	"name": "diffFilter",
	"summary": "Applies QA tools to run on a single pull request",
	"website": "https://github.com/exussum12/coverageChecker",
	"command": {
	"composer-bin-plugin": {
	"package": "exussum12/coverage-checker",
	"namespace": "tools",
	"links": {"%target-dir%/diffFilter": "diffFilter"}
	}
	},
	"test": "diffFilter -v"
	},

This is the only "scoping" we have, but it doesn't really help when your project depends on the same packages as the tool.

In your ticket, @TomasVotruba said:

ECS is only designed as composer dependency or prefixed phar. Global won't work.

If there is a prefixed phar or at least a box config available, we should use it.

[jakzal]

I see they build the phar: https://github.com/symplify/symplify/blob/953e3f49e8679f92fdfbf266d4a7a9ebd0601712/.travis.yml#L20-L45

For some reason, it's not published in github releases or anything like that, but commited to the repository instead: https://github.com/symplify/easy-coding-standard-prefixed

Edit: I understand now. It's so the phar could be installed with composer.

[ostrolucky]

Indeed, we should just use https://github.com/symplify/easy-coding-standard-prefixed. However, this will definitely break some people's CI, as everything is prefixed there, including sniffs/fixers that are installed as dependencies. So references to those in ecs config file will break. But yeah we should still do that and ask to stop prefixing these on their side. I don't have computer with me next couple of days, so can't contribute this now. But if you don't get around to do it and I forget, please remind me in a week or so.

edit: For the reason I mentioned, following is worth checking out first

We could migrate it to composer-bin-plugin, where a namespace can be specified, like here:

[jakzal]

@ostrolucky just sent a PR: #223

Before merging I'd love you to test it if possible. Phars can be downloaded from the build page: https://github.com/jakzal/toolbox/actions/runs/95209465

edit: For the reason I mentioned, following is worth checking out first

We could migrate it to composer-bin-plugin, where a namespace can be specified, like here:

This is really not going to work as all it does it puts the tool into its own global namespace, so it doesn't conflict with other tools. It might conflict with your project though.

PHARs are the best installation format for toolbox. If it's available we should use it really.

Are you sure sniffs/fixers are prefixed? That doesn't sound right.

[ostrolucky]

This is really not going to work as all it does it puts the tool into its own global namespace, so it doesn't conflict with other tools. 

Ah so it's not a composer thing, it doesn't change actual PSR-(0|4) namespaces? Then indeed, that's not going to cut it.

Are you sure sniffs/fixers are prefixed? That doesn't sound right.

I didn't really take a peek inside it's code, but according to errors I got when I tried that package couple days ago in my existing project, that's my explanation. Feedback in comments at https://www.tomasvotruba.com/blog/2020/01/20/introducing-phar-for-easy-coding-standard/#comment-4764517749 also confirm this.

[ostrolucky]

Alright checked again and I was wrong, it doesn't prefix sniffs. My error was because config simply needs to be changed for newer sniffs.