Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace DIRECTORY_SEPARATOR with '/' #69

Merged
merged 1 commit into from
May 11, 2022
Merged

Replace DIRECTORY_SEPARATOR with '/' #69

merged 1 commit into from
May 11, 2022

Conversation

w00key
Copy link
Contributor

@w00key w00key commented May 7, 2022

This pull request aims to fix zips created on windows with a broken folder structure which doesn't match the lambda function handler.

This pull request aims to fix zips created on windows with a broken folder structure which doesn't match the lambda function handler.
@aarondfrancis aarondfrancis merged commit ca923f7 into aarondfrancis:main May 11, 2022
@aarondfrancis
Copy link
Owner

This is a good catch, thank you @w00key!

@ianscotland
Copy link

This did not fix for me as once the removeBasePath() was fixed I still ended up with file patterns that look like /resources/lambda\index.js - I had to add a str_replace to flip the \ to a /

    $str = preg_replace('/^' . preg_quote($this->getBasePath(), '/') . '/i', '', $path);
    return str_replace('\\','/', $str);

@w00key
Copy link
Contributor Author

w00key commented May 17, 2022

Hi @ianscotland can you include your code for your LambdaFunction class and I'll try it?

I tested this with a package that didn't replace the base, its possible a custom base path still has issues.

This was my test package, which mirrored the issue I found about windows.

namespace App\Sidecar\Portals\Pulse;

use Hammerstone\Sidecar\Package;
use Hammerstone\Sidecar\LambdaFunction;

class PulseListTasks extends LambdaFunction {

    public function name() {
        return 'Pulse List Tasks';
    }

    public function handler()
    {
        return 'resources/lambda/portals/pulse/list-tasks.handle';
    }
 
    public function package()
    {
        return Package::make()
        ->include([
            'resources/lambda/portals/pulse/list-tasks.js'
        ]);
    }
}

@ianscotland
Copy link

ianscotland commented May 17, 2022 via email

@w00key
Copy link
Contributor Author

w00key commented May 17, 2022

I'll try making a dummy test function later today using the same package function setup that you are using and see if the array based version is different to my using the Package class directly on windows or the directory itself causes it to go awry.

@w00key
Copy link
Contributor Author

w00key commented May 17, 2022

yeah looks like passing it a directory on windows still has issues with generating a zip, I'll try and see if I can make a similar tweak to my first PR to resolve this issue.

@w00key
Copy link
Contributor Author

w00key commented May 17, 2022

I've submitted a PR which hopefully resolves the issue. #73

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants