Skip to content
This repository has been archived by the owner on Feb 23, 2022. It is now read-only.

Crash when using non-empty input-renaming-map #124

Open
Yannic opened this issue Aug 7, 2017 · 11 comments
Open

Crash when using non-empty input-renaming-map #124

Yannic opened this issue Aug 7, 2017 · 11 comments

Comments

@Yannic
Copy link

Yannic commented Aug 7, 2017

Closure Stylesheets crashes with the following exception when using an non-empty input-renaming-map. The format of the renaming map does not seem to matter, it only must not be empty.

Exception in thread "main" java.lang.ClassCastException: com.google.common.css.IdentitySubstitutionMap cannot be cast to com.google.common.css.SubstitutionMap$Initializable
	at com.google.common.css.RecordingSubstitutionMap.initializeWithMappings(RecordingSubstitutionMap.java:91)
	at com.google.common.css.compiler.passes.PassRunner.createSubstitutionMap(PassRunner.java:213)
	at com.google.common.css.compiler.passes.PassRunner.<init>(PassRunner.java:49)
	at com.google.common.css.compiler.commandline.DefaultCommandLineCompiler.<init>(DefaultCommandLineCompiler.java:76)
	at com.google.common.css.compiler.commandline.ClosureCommandLineCompiler.<init>(ClosureCommandLineCompiler.java:65)
	at com.google.common.css.compiler.commandline.ClosureCommandLineCompiler.executeJob(ClosureCommandLineCompiler.java:371)
	at com.google.common.css.compiler.commandline.ClosureCommandLineCompiler.main(ClosureCommandLineCompiler.java:440)

Repo:
java -jar closure-stylesheets-1.5.0-SNAPSHOT-jar-with-dependencies.jar --input-renaming-map renaming.json some.css

renaming.json

{
  "foo": "bar"
}

some.css
Content does not matter, can be empty.

Yannic added a commit to Yannic/closure-stylesheets that referenced this issue Aug 27, 2017
@elipoultorak
Copy link

Any reason this wasn't merged yet? The problem still exists, and @Yannic's commit fixes it.

@sgammon
Copy link

sgammon commented Jul 2, 2019

@elipoultorak it does fix it but if memory serves, it just disables the input renaming map? or did it work for you? if so then yeah why isn't this merged lol

@Yannic
Copy link
Author

Yannic commented Jul 2, 2019

TBH, I don't remember what problem I was trying to solve when I initially run into this crash, and I haven't seen it for the last two years.
IIRC, #125 worked but wasn't merged because the "fix" just hides the issue that the map isn't initialized where that's supposed to happen.

@elipoultorak
Copy link

I just tried it, and for some reason, it does rename it properly with the fix. I used a CLOSURE_COMPILED format instead of json, so I don't know if it would work with json. But when I use the master branch, or the last release, it gives me an error. When I use @Yannic's fork, it uses the correct renaming according to the map.
I know it sounds weird, do you know why it happens? Maybe the SubstitutionMap is initialised more than once, sometimes using a correct class, and sometimes not?

@elipoultorak
Copy link

Update: it also works with json maps.

@Yannic
Copy link
Author

Yannic commented Jul 3, 2019

AFAIK, the crash above happens because the map isn't initialized at all. You're right that my fix works, but the map is only initialized right before the crash would happen and not where the initialization is supposed to happen.
Unfortunately, that's all I remember right now. If I remember to do so, I'll have a look (next week or so) if I can find some notes from two years ago.

@elipoultorak
Copy link

That makes more sense, but it doesn't explain why the code works. :-)

my-code-doesnt-work-i-have-noidea-why-my-code-33594653

@sgammon
Copy link

sgammon commented Jul 3, 2019

@elipoultorak @Yannic sick, ha ha
another one bites the dust, good work @Yannic

@sgammon
Copy link

sgammon commented Jul 3, 2019

i have a fork of closure-stylesheets that gathers most, if not all, of these fixes and features introduced via PRs. i'll merge this in today, feel free to ping me if our fork can be helpful to you in the meantime before this gets merged to master upstream.

sgammon added a commit to Bloombox/closure-stylesheets that referenced this issue Jul 3, 2019
@elipoultorak
Copy link

Looks good, but is nobody at Google merging anything to this repo?

@sgammon
Copy link

sgammon commented Jul 3, 2019

@elipoultorak welcome to the gap between regular open source, and google internal :) lol

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

No branches or pull requests

3 participants