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

integrate github.com/pmezard/go-difflib #1198

Closed
wants to merge 3 commits into from

Conversation

obsti8383
Copy link

Summary

Since go-difflib is unmaintained since quite some time, the required functions have been taken over into a separate testify package.

Motivation

Unmaintained packages might vanish or be taken over by attackers.

Related issues

Closes #1187
Closes #1159
Closes #736

@boyan-soubachov
Copy link
Collaborator

It's a good idea but would it not be easier to just pin to the latest version in our go.mod file and break on any hash changes (go.sum, we already do that in the CI jobs).

I get the problem of what happens if the package vanishes, would vendoring not be an easier solution then?

@obsti8383
Copy link
Author

It's a good idea but would it not be easier to just pin to the latest version in our go.mod file and break on any hash changes (go.sum, we already do that in the CI jobs).

go.mod already PINs the version, so that doesn't change anything. Pulling everything into this repo gives the advantage that you don't upgrade to a malicious version by accident.

Main advantage of moving all used code into this repo is that you get rid of concerns by using an unmaintained source code, by maintaining it yourself (see all three issues).

I've also included only the used functions, which makes the code base smaller.

I get the problem of what happens if the package vanishes, would vendoring not be an easier solution then?

As far as I understand vendoring it doesn't bring any advantages here, since command like "go get" will still use the original repository and you don't get rid of the dependency.

Copy link

@miguelalcantar miguelalcantar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything seems good to me. Also I need this upgrade

@miguelalcantar
Copy link

miguelalcantar commented Oct 5, 2022

@boyan-soubachov @ernesto-jimenez, could we have a look at this?

@ilkka
Copy link

ilkka commented Mar 3, 2023

Would love to see this integrated as well.

@TReadOnly
Copy link

Looking forward to having github.com/pmezard/go-difflib integrated.

@bmv126
Copy link

bmv126 commented Jul 18, 2023

Any update on this ?
Can this be merged.

@dolmen dolmen added dependencies Pull requests that update a dependency file internal/refactor Refactor internals with no external visible changes rejected/invalid Not a bug but a misunderstanding by the requester labels Oct 30, 2023
@dolmen
Copy link
Collaborator

dolmen commented Oct 30, 2023

We have enough code to maintain and not enough maintainer.

We should instead investigate using another library that provides the same features.

@dolmen dolmen closed this Oct 30, 2023
@bmv126
Copy link

bmv126 commented Nov 12, 2023

How about https://github.com/martinohmann/go-difflib ?

Copy link
Collaborator

@dolmen dolmen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move package difflib to internal/difflib.

@dolmen
Copy link
Collaborator

dolmen commented Nov 19, 2023

@miguelalcantar Why do you "need" that change?

@obsti8383
Copy link
Author

Move package difflib to internal/difflib.

@dolmen You closed this PR 3 weeks ago. What do you mean?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file internal/refactor Refactor internals with no external visible changes rejected/invalid Not a bug but a misunderstanding by the requester
Projects
None yet
7 participants