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

Fix shading #317

Merged
merged 2 commits into from
Mar 10, 2024
Merged

Fix shading #317

merged 2 commits into from
Mar 10, 2024

Conversation

lifebarier
Copy link
Contributor

Seems to fix shading issue #310 #316

@Stewori
Copy link
Contributor

Stewori commented Mar 5, 2024

Thank you for this work. The fix looks good to me and makes sense to me. It did not occur to me that a module-info file is in the game because it is so well hidden and not present in source.
Have you signed the CLA?
Also, please feel free to add yourself to ACKNOWLEDGMENTS (unless you are already in the list).

@lifebarier
Copy link
Contributor Author

Just signed CLA. I will skip ACKNOWLEDGMENTS file for now.

@Stewori
Copy link
Contributor

Stewori commented Mar 6, 2024

Alright. (We may still record your contribution in ACKNOWLEDGMENTS eventually by means of #308, I hope that's okay for you.)

@Stewori
Copy link
Contributor

Stewori commented Mar 6, 2024

@juniartisu Does this fix work for you as well?/Can you confirm it fixes #316 in your environment?

@juniartisu
Copy link

@Stewori I will try out the fix and do testing on it and will put update here once it is done.

@juniartisu
Copy link

@Stewori The fix is good. I am not seeing any issue so far. Than you @lifebarier for investigating and fixing it.

@Stewori
Copy link
Contributor

Stewori commented Mar 9, 2024

@lifebarier could you do me a favor and add the line
- [ GH-316 ] PR313 cause regression
to the NEWS file (line 22) as part of this PR? Then I wouldn't have to make a separate PR for that. When done, I will merge this PR. Thanks again!

@Stewori Stewori merged commit f0afabb into jython:master Mar 10, 2024
8 checks passed
@abhi-rck
Copy link

abhi-rck commented Mar 21, 2024

@lifebarier @Stewori
i tried to build the jar with ant after this fix. Though the shading is done properly but still i see errors when i am trying to decompile the built jar with Java Decompiler jd-gui-1.6.6.jar.
this error indicate incomplete or corrupted jar.

Exception in thread "AWT-EventQueue-0" java.lang.ArrayIndexOutOfBoundsException: 9
at com.sun.nio.zipfs.ZipConstants.SH(ZipConstants.java:192)
at com.sun.nio.zipfs.ZipConstants.LG(ZipConstants.java:196)
at com.sun.nio.zipfs.ZipFileSystem$Entry.readExtra(ZipFileSystem.java:2302)
at com.sun.nio.zipfs.ZipFileSystem$Entry.cen(ZipFileSystem.java:1903)
at com.sun.nio.zipfs.ZipFileSystem$Entry.readCEN(ZipFileSystem.java:1871)
at com.sun.nio.zipfs.ZipFileSystem.getEntry0(ZipFileSystem.java:1329)
at com.sun.nio.zipfs.ZipFileSystem.getFileAttributes(ZipFileSystem.java:317)
at com.sun.nio.zipfs.ZipPath.getAttributes(ZipPath.java:664)
at com.sun.nio.zipfs.ZipFileSystemProvider.readAttributes(ZipFileSystemProvider.java:294)
at java.nio.file.Files.readAttributes(Files.java:1737)
at java.nio.file.Files.isDirectory(Files.java:2192)
at org.jd.gui.model.container.GenericContainer$Entry.isDirectory(GenericContainer.java:110)
at org.jd.gui.model.container.ContainerEntryComparator.compare(ContainerEntryComparator.java:26)

@lifebarier
Copy link
Contributor Author

@abhi-rck
Why are you trying to decompile jar?
I would be really surprised if change to exclude 2 module-info files from build would break jar in such way.
Anyhow, for fun I tired to open jython-standalone.jar with that decompiler and it opened just fine.

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.

None yet

4 participants