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

libsass 3.4: file context fails on missing input path, event though it is provided in constructor #2257

Closed
gauteh opened this issue Dec 20, 2016 · 8 comments

Comments

@gauteh
Copy link

gauteh commented Dec 20, 2016

Using file context with libsass based on example fails on libsass 3.4

Hi,

I am using libsass based on the example provided for the file context here. However, I've had multiple reports of breakage (astroidmail/astroid#248, astroidmail/astroid#244) with libsass 3.4.0 (default on Debian and Arch Linux), getting the error:

Internal Error: File context has no input path

I am loading sass in this way (https://github.com/astroidmail/astroid/blob/master/src/modes/thread_view/theme.cc#L96):

    const char * scsspath = "path/to/file.scss";

    struct Sass_File_Context* file_ctx = sass_make_file_context(scsspath);
    struct Sass_Options* options = sass_file_context_get_options(file_ctx);
    struct Sass_Context* context = sass_file_context_get_context(file_ctx);
    sass_option_set_precision(options, 1);
    sass_option_set_source_comments(options, true);

    sass_file_context_set_options(file_ctx, options);

    int status = sass_compile_file_context (file_ctx);

    if (status != 0) {
      const char * err = sass_context_get_error_message (context);
      ustring erru (err);
      LOG (error) << "theme: error processing: " << erru;

      throw runtime_error (
          ustring::compose ("theme: could not process scss: %1", erru).c_str ());
    }

    const char * output = sass_context_get_output_string(context);
    ustring output_str(output);
    sass_delete_file_context (file_ctx);

    return output_str;

version info:

libsass         3.4.0   (Sass Compiler) [C/C++]
@gauteh
Copy link
Author

gauteh commented Dec 20, 2016

From looking at git blame the culprit might be 6a5444b, if I remove the call to sass_file_context_set_options the error is lost. I cannot inspect the structs more in depth using gdb at this point since I do not have access to the complete type in gdb.

Adding a sass_option_set_input_path (options, scsspath); makes no difference.

gauteh added a commit to astroidmail/astroid that referenced this issue Dec 20, 2016
recent libsass (3.4) resets the original context when setting an option
(sass/libsass#2257) causing the input_path to
be lost (#244). here we set the options directly on file_ctx which is a
subclass of options - this might break in the future if file_ctx no
longer sub-classes options. oh well.
@mgreter
Copy link
Contributor

mgreter commented Dec 20, 2016

@gauteh the commit you mention (6a5444b) is not part of libsass 3.4.0? So I'm confused if your report is against master or libsass 3.4.0 (as you stated in your first post)? Also does your example above exhibit the problem or is that just for illustration? Thanks!

@gauteh
Copy link
Author

gauteh commented Dec 20, 2016

@mgreter Ah - sorry, I did not notice that. The problem is still with libsass 3.4.0. The example exhibits the problem, If I change it to (astroidmail/astroid@b44b9fc):

    struct Sass_File_Context* file_ctx = sass_make_file_context(scsspath);
    struct Sass_Context* context = sass_file_context_get_context(file_ctx);
    sass_option_set_precision((Sass_Options*) file_ctx, 1);
    sass_option_set_source_comments((Sass_Options*) file_ctx, true);

    int status = sass_compile_file_context (file_ctx);

    if (status != 0) {
      const char * err = sass_context_get_error_message (context);
      ustring erru (err);
      LOG (error) << "theme: error processing: " << erru;

      throw runtime_error (
          ustring::compose ("theme: could not process scss: %1", erru).c_str ());
    }

    const char * output = sass_context_get_output_string(context);
    ustring output_str(output);
    sass_delete_file_context (file_ctx);

    return output_str;
  }

it works.

(on a side note: should I free output here? or is it freed by delete_file_context?)

@mgreter
Copy link
Contributor

mgreter commented Dec 20, 2016

OK, I was able to verify the issue on linux. Could not reproduce on windows with mingw64 and msys2.

@mgreter
Copy link
Contributor

mgreter commented Dec 20, 2016

Can you try out the fix in #2259 ?

Btw. your code can also be fixed to work with current version:

    struct Sass_File_Context* file_ctx = sass_make_file_context(scsspath);
    struct Sass_Context* context = sass_file_context_get_context(file_ctx);
    struct Sass_Options* options = sass_file_context_get_options(file_ctx);
    sass_option_set_precision(options, 1);
    sass_option_set_source_comments(options, true);

-    sass_file_context_set_options(file_ctx, options);

    int status = sass_compile_file_context (file_ctx);

No need to set options again. They are infact the same object. You mostly just need that function if you create options via sass_make_options (which can be usefull if you want to set it up before file or data contexts). If you look at sass_file_context_get_options you'll find it only does return (Sass_Options*) this.

@gauteh
Copy link
Author

gauteh commented Dec 20, 2016

Hi @mgreter, thanks - your proposed fix works fine (and should be portable if you decide to change the class structure later on). I do not have libsass set up in a development environment at the moment, so I won't have the time to test #2259 now unfortunately. It does look like it would fix the issue though.

gauteh added a commit to astroidmail/astroid that referenced this issue Dec 20, 2016
recent libsass (3.4) deletes the original context when setting an option
(sass/libsass#2257). here we get the options,
and set the option without re-setting the options object.
@mgreter
Copy link
Contributor

mgreter commented Dec 21, 2016

Closing as resolved

@mgreter mgreter closed this as completed Dec 21, 2016
@gauteh
Copy link
Author

gauteh commented Dec 21, 2016 via email

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

No branches or pull requests

3 participants