chore: remove buildkit dependency #2864
Conversation
efekarakus
requested review from
iamhopaul123
and removed request for
a team
iamhopaul123
left a comment
iamhopaul123
left a comment
There was a problem hiding this comment.
This is a really mind-blowing PR! Thank you for sharing this idea 🎉
|
|
||
| // parse parses the contents of a Dockerfile into a Dockerfile struct. | ||
| func parse(content string) (*Dockerfile, error) { | ||
| func parse(name, content string) (*Dockerfile, error) { |
There was a problem hiding this comment.
Maybe
| func parse(name, content string) (*Dockerfile, error) { | |
| func parse(lexer *lexer) (*Dockerfile, error) { |
There was a problem hiding this comment.
Kept it as is because i removed name from the lexer following Wanxian's comment, so now I'm writing the name information in this function!
|
|
||
| const ( | ||
| exposeInstrMarker = "expose " // start of an EXPOSE instruction. | ||
| heathCheckInstrMarker = "healthcheck " // start of a HEALTHCHECK instruction. |
There was a problem hiding this comment.
nit:
| heathCheckInstrMarker = "healthcheck " // start of a HEALTHCHECK instruction. | |
| healthCheckInstrMarker = "healthcheck " // start of a HEALTHCHECK instruction. |
There was a problem hiding this comment.
lol oops, changed the const names to markerExposeInstr and markerHealthCheckInstr
| idx := strings.Index(normalized, instrMarker) + len(instrMarker) | ||
| return line[idx:] |
There was a problem hiding this comment.
TrimPrefix as is wouldn't work because it's possible that the instruction has leading spaces, for example:
EXPOSE 8080
But good point! to be safe with the idx I added the following guard-statement:
if !strings.Contains(normalized, instrMarker) {
return line
}| func trimContinuationLineMarker(line string) string { | ||
| for _, marker := range lineContinuationMarkers { | ||
| if strings.HasSuffix(line, marker) { | ||
| return line[:len(line)-1-len(marker)] |
|
|
||
| // trimLeadingWhitespaces removes any leading space characters. | ||
| func trimLeadingWhitespaces(line string) string { | ||
| return strings.TrimLeftFunc(line, unicode.IsSpace) |
There was a problem hiding this comment.
maybe https://pkg.go.dev/strings#TrimSpace to trim both leading and trailing spaces?
There was a problem hiding this comment.
I mostly just followed the logic from buildkit, so I'm hesitant to change it in case it results in poor parsing
| ) | ||
|
|
||
| const ( | ||
| exposeInstrMarker = "expose " // start of an EXPOSE instruction. |
There was a problem hiding this comment.
or instrMarkerExpose and instrMarkerHealthcheck?
There was a problem hiding this comment.
renamed to markerExposeInstr and markerHealthCheckInstr!
| } | ||
|
|
||
| // stateFn represents a state machine transition of the scanner going from one INSTRUCTION to the next. | ||
| type stateFn func(*lexer) stateFn |
| func (lex *lexer) readLine() (isEOF bool, err error) { | ||
| if ok := lex.scanner.Scan(); !ok { | ||
| if err := lex.scanner.Err(); err != nil { | ||
| return false, fmt.Errorf("scan Dockerfile %s: %w", lex.name, err) |
There was a problem hiding this comment.
I wonder if the lex's error cares about the file name information 🤔 If lex is just scanning and processing the content, maybe the file name information would be better suited in the error that wraps this error, that is in the caller error? Just a thought 💭
There was a problem hiding this comment.
Good point! removed
| func trimContinuationLineMarker(line string) string { | ||
| for _, marker := range lineContinuationMarkers { | ||
| if strings.HasSuffix(line, marker) { | ||
| return line[:len(line)-1-len(marker)] |
There was a problem hiding this comment.
Is the extra -1 for space " "?
There was a problem hiding this comment.
omg that was a bug! The test cases passed because all my line-wraps had an extra space abc \ instead of abc\.
I've added a new test case and removed the -1
| } | ||
|
|
||
| // stateFn represents a state machine transition of the scanner going from one INSTRUCTION to the next. | ||
| type stateFn func(*lexer) stateFn |
There was a problem hiding this comment.
computer science working 💻 🔮
!!!
Removing the dependency saves us 3.2MB of space and brings us down to 42MB. The bulk of the lexer is heavily inspired by https://cs.opensource.google/go/go/+/refs/tags/go1.17.1:src/text/template/parse/lex.go By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.
5 participants
Removing the dependency saves us 3.2MB of space and brings us down to 42MB.
The bulk of the lexer is heavily inspired by https://cs.opensource.google/go/go/+/refs/tags/go1.17.1:src/text/template/parse/lex.go
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.