search

pharo-project / pharo

Pharo is a dynamic reflective pure object-oriented language supporting live programming inspired by Smalltalk.
http://pharo.org
Other
1.21k stars 355 forks source link

Reconsider LanguageEnvironment and Locale[ID] in minimal image #3844

Closed jgfoster closed 1 year ago

jgfoster commented 5 years ago

In #3832 @noha suggested that LanguageEnvironment be removed from the minimal image. This issue is available for that discussion (and so that the earlier PR can proceed).

welcome[bot] commented 5 years ago

Thanks for opening your first issue! Please check the CONTRIBUTING documents for some tips about which information should be provided. You can find information of how to do a Pull Request here: https://github.com/pharo-project/pharo/wiki/Contribute-a-fix-to-Pharo

GitHub
pharo-project/pharo
The Sources for Pharo. Contribute to pharo-project/pharo development by creating an account on GitHub.
noha commented 5 years ago

As with other packages we need to analyze dependencies and the potential rearrangement of the load order and removal from the minimal build

bencoman commented 5 years ago

The term "minimal" is open to interpretation. There is:

super minimal, really the tiniest it can be made minimal, I want to fit on an embedded microprocessor (someday) minimal, want to run remotely a web service and don't want any graphics baggage The first might not want LanguageEnvironment while the latter probably does. Perhaps in a while we need a separate bit less minimal image called "headless" to match the "headless vm".

MarcusDenker commented 5 years ago

PR was merged

jecisc commented 5 years ago

@MarcusDenker This issue is about something else than what was done in the PR.

jgfoster commented 5 years ago

It appears that ZipArchive depends on Locale:

LanguageEnvironment class>>currentPlatform
LanguageEnvironment class>>defaultSystemConverter
String>>convertToSystemString
ZipArchiveMember>>writeCentralDirectoryFileHeaderTo:
ZipArchive>>writeCentralDirectoryTo:

So, I'll try bringing the System-Localization package into the minimal image.

guillep commented 5 years ago

I'm checking the Zip implementation there:

writeCentralDirectoryFileHeaderTo: aStream
    "C2 v3 V4 v5 V2"

    | systemFileName systemFileComment systemCdExtraField endianStream |
    systemFileName := fileName asVmPathName.
    systemFileComment := fileComment convertToSystemString.
    systemCdExtraField := cdExtraField.
    aStream nextPutAll: CentralDirectoryFileHeaderSignature.

    endianStream := ZnEndianessReadWriteStream on: aStream.
    endianStream nextLittleEndianNumber: 1 put: self versionMadeBy.
    endianStream nextLittleEndianNumber: 1 put: fileAttributeFormat.

    endianStream nextLittleEndianNumber: 2 put: versionNeededToExtract.
    endianStream nextLittleEndianNumber: 2 put: self bitFlag.
    endianStream nextLittleEndianNumber: 2 put: desiredCompressionMethod.

    endianStream nextLittleEndianNumber: 4 put: lastModFileDateTime asDosTimestamp.

    "These next 3 should have been updated during the write of the data"
    endianStream nextLittleEndianNumber: 4 put: crc32.
    endianStream nextLittleEndianNumber: 4 put: (desiredCompressionMethod = CompressionStored
                                                ifTrue: [ uncompressedSize ] ifFalse: [ compressedSize ]).
    endianStream nextLittleEndianNumber: 4 put: uncompressedSize.

    endianStream nextLittleEndianNumber: 2 put: systemFileName size.
    endianStream nextLittleEndianNumber: 2 put: systemCdExtraField size.
    endianStream nextLittleEndianNumber: 2 put: systemFileComment size.
    endianStream nextLittleEndianNumber: 2 put: 0.      "diskNumberStart"
    endianStream nextLittleEndianNumber: 2 put: internalFileAttributes.

    endianStream nextLittleEndianNumber: 4 put: externalFileAttributes.
    endianStream nextLittleEndianNumber: 4 put: writeLocalHeaderRelativeOffset.

    aStream nextPutAll: systemFileName asByteArray.
    aStream nextPutAll: systemCdExtraField asByteArray.
    aStream nextPutAll: systemFileComment asByteArray

And I wonder whether the offending line (fileComment convertToSystemString) really requires the system's encoding... We should maybe dig on the Zip specs because maybe it requires that we somehow tell the encoding? Otherwise the results may not be "unzippable" in a different machine with different encoding.

guillep commented 5 years ago

Wikipedia talks about encoding file names in utf8 (see https://en.wikipedia.org/wiki/Zip_(file_format)#Data_descriptor). But nothing on the comments.

Zip (file format) - Wikipedia
guillep commented 5 years ago

Appendix D in https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT specifies that a bit can be set to 1 to specify the encoding used is UTF8, which is IMHO the safest thing to do...

guillep commented 5 years ago

Also to document this a bit better, there are no senders of fileComment: and the only assignments to it set an empty string during initialization fileComment := ''.

stale[bot] commented 4 years ago

To limit bug bankruptcy (see https://www.joelonsoftware.com/2012/07/09/software-inventory/) this issue has been automatically marked as stale because it has not had any activity in 6 months. If no further activity occurs, it might be closed. If this issue remains important to you, please comment to reactivate the issue. Thank you for your contributions.

Joel on Software
Software Inventory
Imagine, for a moment, that you came upon a bread factory for the first time. At first it just looks like a jumble of incomprehensible machinery with a few people buzzing around. As your eyes adjus…
astares commented 4 years ago

Still relevant

Ducasse commented 4 years ago

Indeed!

guillep commented 1 year ago

This issue (the original one) looks more like a small project than an actionable issue. I propose we move it to a wiki with potential projects for people to contribute. I'm collecting all these. Could be a great basis for GSOC.

The cleanup to ZipArchive is on the other hand easily actionable. I'll open another issue for it.