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

Invalid compiler command is dispatched in the is_outdated #14

Closed
MrCsabaToth opened this issue Jul 8, 2016 · 8 comments
Closed

Invalid compiler command is dispatched in the is_outdated #14

MrCsabaToth opened this issue Jul 8, 2016 · 8 comments

Comments

@MrCsabaToth
Copy link

MrCsabaToth commented Jul 8, 2016

This https://github.com/j0hnsmith/django-pipeline-browserify/blob/master/pipeline_browserify/compiler.py#L55 yields a single string u' c:\\Users\\JohnSmith\\node_modules\\.bin\\browserify.cmd -t babelify --deps C:\\Users\\JohnSmith\\Documents\\test\\company\\static\\dashboard\\js\\react_test_dashboard_widget.browserify.js' on my system. That causes a disaster in the pipeline SubProcessCompiler since that expects a tuple. Upon receiving a string it tears it up into hundred individual characters. So instead of

        command = "%s %s %s --deps %s" % (
            getattr(settings, 'PIPELINE_BROWSERIFY_VARS', ''),
            getattr(settings, 'PIPELINE_BROWSERIFY_BINARY', '/usr/bin/env browserify'),
            getattr(settings, 'PIPELINE_BROWSERIFY_ARGUMENTS', ''),
            self.storage.path(infile),
        )

we'd need something like

        command = (
            getattr(settings, 'PIPELINE_BROWSERIFY_BINARY', '/usr/bin/env browserify'),
            getattr(settings, 'PIPELINE_BROWSERIFY_ARGUMENTS', ''),
            "--deps %s" % self.storage.path(infile),
        )

How does this work for others at all?

@MrCsabaToth
Copy link
Author

The part which doesn't work well when the passed command is not a tuple but a long string: https://github.com/jazzband/django-pipeline/blob/master/pipeline/compilers/__init__.py#L108

    argument_list = []
    for flattening_arg in command:
        if isinstance(flattening_arg, string_types):
            argument_list.append(flattening_arg)
        else:
            argument_list.extend(flattening_arg)

This would lead to:

CompilerError: [Error 87] The parameter is incorrect

@MrCsabaToth
Copy link
Author

@MrCsabaToth
Copy link
Author

I'm still doing something wrong, since the compilation errors out though: https://stackoverflow.com/questions/38276360/using-django-pipeline-browserify-on-windows

@MrCsabaToth
Copy link
Author

Ooooh, it was changed around in September 2015, before that the long string was valid: jazzband/django-pipeline@1f6b48a

@MrCsabaToth
Copy link
Author

I tried all kinds of combinations to assemble the command. Like leaving the original long string but use split() when passing it to the executor, just like with the https://github.com/j0hnsmith/django-pipeline-browserify/blob/master/pipeline_browserify/compiler.py#L27. My colleague with Mac also couldn't get it to work, so finally we just short circuited the is_outdated to True (plugging in our own compiler).

Something needs to be done about this.

@j0hnsmith
Copy link
Owner

j0hnsmith commented Jul 12, 2016

I haven't used this project since I first created it so I'm not really familiar with how it works now. I've merged a few pull requests which may have had unintended consequences.

When I get a chance I'll setup a dev environment so I try out the latest version of the code.

@solkaz
Copy link
Contributor

solkaz commented Aug 30, 2016

@MrCsabaToth @j0hnsmith I've fixed this issue with this PR

@j0hnsmith
Copy link
Owner

Fixed via #16

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

No branches or pull requests

3 participants