hiqdev / asset-packagist

Asset Packagist
https://asset-packagist.org
BSD 3-Clause "New" or "Revised" License
247 stars 24 forks source link

Bower packages are case sensitive #26

Closed MaksimKiselev closed 4 years ago

MaksimKiselev commented 7 years ago

Please see https://github.com/fxpio/composer-asset-plugin/issues/70

asset-packagist also are case sensitive :worried:

hiqsol commented 7 years ago

Please show me the composer.json which is in trouble.

MaksimKiselev commented 7 years ago

@hiqsol I was add in require section bower-asset/Sortable and expect that will be downloaded https://github.com/RubaXa/Sortable but asset-packagist was download bower-asset/sortable https://github.com/HubSpot/sortable

githubjeka commented 7 years ago

same here :(

"require": {        
        "bower-asset/At.js": "^1.5.1",
    },
SilverFire commented 7 years ago

Seems to be annoying. Will try to dedicate some time and fix it

SilverFire commented 7 years ago

Composer is case-insensitive, so package name, listed in require section will be lowercased to install.

We can create aliases for packages, that are not lowercase to distinguish bower-asset/At.js from bower-asset/at.js. For example, we can add hash to the package name:

$hash = substr(md5('bower-asset/At.js'), 0, 6); // 8fe67c

Then use this part of hash in the package name, for example bower-asset/At.js will be aliased to bower-asset/at.js-8fe67c.

Asset-packagist will save this alias and it's package representation and provide correct package.

The problem I see for now is that package will be saved locally in directory vendor/bower-asset/at.js-8fe67c so we need either to use oomphinc/composer-installers-extender and map this directory back to vendor/bower-asset/at.js or use it as-is. In case we use it as-is, we create logical dependency on asset-packagist with the package name.

I can't see another way how we can overcome this problem, at least for now. In case anyone have better ideas - I will be glad to hear your thoughts.

@francoispluchino

edgardmessias commented 7 years ago

My idea: ASCII table

Example:

"bower-asset/At.js"    => "bower-asset/At.js"
"bower-asset/Sortable" => "bower-asset/Sortable"

I'm doing a local test.

SilverFire commented 7 years ago

It looks much less readable

rob006 commented 7 years ago

We could use double underscore: bower-asset/__at.js. Or any special char, like bower-asset/^at.js.

SilverFire commented 7 years ago

Before each uppercase letter?

rob006 commented 7 years ago

Yes.

francoispluchino commented 7 years ago

@SilverFire The problem will not be resolved for the dependencies of the root package. The Solver SAT names the package in lowercase before the search. Therefore, all Composer repositories receive a package name in lowercase.

Technically, nothing blocks to edit Composer and allow some packages to be case sensitive, But lead developers of Composer do not want to accept this change, it's all the more frustrating that there have not much change (search the function strtolower()). It was to 2 years ago, maybe it's changed now.

SilverFire commented 7 years ago

The Solver SAT names the package in lowercase before the search. Therefore, all Composer repositories receive a package name in lowercase.

That's right, but your composer-asset-plugin fetches non-lowercase packages from Bower/NPM without any problem: https://asset-packagist.org/p/bower-asset/Blueprint-SlidePushMenus/latest.json

As it was suggested before, we can provide these packages in lowercase with mutated name as it was suggested above:

P.S.: I don't thing Jordi will change something in Composer for this problem :)

francoispluchino commented 7 years ago

Yes it's the case, but the package name is normalized in Composer. So, if you have 2 packages with the same name but with a different case, Composer will consider that it is the same package in the resolution of dependencies. Therefore, it is not certain that we install the right package.

Non-exhaustive list of calls to the strtolower function which is problematic:

PS. It was mainly Nils who did not want to modify the Solver SAT.

rob006 commented 7 years ago

@francoispluchino But if in require section of my composer.json I will use bower-asset/^at.js instead of bower-asset/At.js are there any technical problems with treating bower-asset/^at.js as At.js bower package? For composer it will be bower-asset/^at.js so strtolower is irrelevant. It will not be 100% transparent to users, but at least it works.

francoispluchino commented 7 years ago

@rob006 Why not! If you have a solution, can you do a PR on fxpio/composer-asset-plugin? Did you check if the special characters are accepted by Composer?

rob006 commented 7 years ago

Did you check if the special characters are accepted by Composer?

According to docs there are no limitations, only recommendations. Bower does not allow ^ in package name, so there should be no conflicts (at least in theory).

Unfortunately, I don't have time to implement it myself.

edgardmessias commented 7 years ago

I started working

dsnopek commented 7 years ago

It seems to be impossible to require any packages with uppercase letters, even if there is no conflict. For example, I'm trying to do:

{
  "require": {
     "bower-asset/angular-unsavedChanges": "~0.1.1"
  }
}

... and I get this error:

  Problem 1
    - The requested package bower-asset/angular-unsavedchanges could not be found in any version, there may be a typo in the package name.
totten commented 7 years ago

It feels like uppercase characters aren't very common in bower package names -- but the numbers matter. If 20% of packages have uppercase, then it's a big problem. If 0.5% of packages, then downstream folks could look for workarounds (eg don't use a particular asset, or download it manually, or ask upstream to rename).

To get a ballpark, I wrote a quick and dirty script. Overall, ~6% of bower packages seem to have an uppercase letter. The distribution varies a bit depending on what kind of packages you look at, e.g.

hiqsol commented 7 years ago

I have a simple but a controversial fix #64 Please, see and comment.

github-actions[bot] commented 4 years ago

Stale issue message