UpliftGames / wally

Wally is a modern package manager for Roblox projects inspired by Cargo
https://wally.run
Mozilla Public License 2.0
317 stars 105 forks source link

Include does not actually whitelist files or respect .gitignore #70

Open AmberGraceRblx opened 2 years ago

AmberGraceRblx commented 2 years ago

I'm trying to publish Pract (https://github.com/ambers-careware/pract/tree/wally-package) as a wally package, but I don't want to include things such as the unit test environment, and only want to include the "Pract" folder.

gitignore: image

wally.toml: image

What actually ends up being installed with the package: image

Small edit after writing this: I decided to instead use a shell file to copy the Pract folder and automate the process of publishing the package from a temporary directory, as I don't actually want to include wally.toml in my repository's source files.

OverHash commented 2 years ago

I also experienced this issue, so I did a bit of digging. When I had the following wally configuration:

include = [
    "src/init.lua",
    "src/commify.lua",
    "src/numbersToSortedString.lua",
    "src/numberToString.lua",
    "src/setSetting.lua",
    "src/stringToNumber.lua",
]
exclude = ["*"]

using

wally package --list

will output no files to be packaged.

The current documentation for include does not reflect the present behavior (https://github.com/UpliftGames/wally/blob/6c984ea171cb9fbd22a1c1e5c183c925acb1ddce/src/manifest.rs#L99-L110)

When I remove the exclude = ["*"], I will get an output alike:

./foreman.toml
./package-lock.json
./package.json
./README.md
./roblox.toml
./selene.toml
./src
./testez.toml
./tests.project.json
./vendor
./wally.toml

which is a lot more than I initially specified!

This issue steps from the following code in package_content.rs: https://github.com/UpliftGames/wally/blob/6c984ea171cb9fbd22a1c1e5c183c925acb1ddce/src/package_contents.rs#L120-L135 If includes is not empty (that is, the first part of the if statement evaluates to true) but the second part does not evaluate to true, then the code will continue to check against the exclude globs, which is not the desired behavior. Instead, it should be removed from the list of files to include.

Something more like

// check that `includes` is not empty
// and that the element is inside the include list (non-empty)
if !includes.is_empty() && include.matches(relative).is_empty() {
    // element was not inside the include list, it should not
    // be included in the uploaded files
    return false;
}

// check that file was not in the exclude list
exclude.matches(relative).is_empty()

would be more suitable here.