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

Remove tar support #1111

Merged
merged 1 commit into from
Nov 11, 2019
Merged

Remove tar support #1111

merged 1 commit into from
Nov 11, 2019

Conversation

Morganamilo
Copy link
Contributor

tar is slower and harder to diff. I doubt any one still uses it intentially.

There's a chance some people have not cleared their cache in two years
and still have tar based packages around. But forcing them to clear
their cache is not a big deal at all.

@Morganamilo Morganamilo closed this Nov 4, 2019
@Morganamilo Morganamilo reopened this Nov 4, 2019
@Morganamilo
Copy link
Contributor Author

I ended up removing -f but looks like we still need it for ABS packages. Even though we use git now we still delete the dir and copy it over.

@Morganamilo
Copy link
Contributor Author

Morganamilo commented Nov 4, 2019

Also reading through the ABS code, do we really need to shell out to cp?

@Jguer
Copy link
Owner

Jguer commented Nov 4, 2019

Also reading through the ABS code, do we really need to shell out to cp?

A inheritance from the old inline tar.gz code, can be removed.
https://stackoverflow.com/questions/51779243/copy-a-folder-in-go

@Jguer
Copy link
Owner

Jguer commented Nov 4, 2019

#1102 now implements the changes to bash and zsh completion for this

@@ -528,10 +526,6 @@ func handleConfig(option, value string) bool {
config.AnswerUpgrade = value
case "noanswerupgrade":
config.AnswerUpgrade = ""
case "gitclone":
config.GitClone = true
case "nogitclone":
Copy link
Owner

Choose a reason for hiding this comment

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

Also missing removal of case "tar" in both attribution and valid param

@@ -72,7 +72,6 @@ type Configuration struct {
NoConfirm bool `json:"-"`
Devel bool `json:"devel"`
CleanAfter bool `json:"cleanAfter"`
GitClone bool `json:"gitclone"`
Copy link
Owner

Choose a reason for hiding this comment

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

Missing removal of config.Tarbin

tar is slower and harder to diff. I doubt any one still uses it intentially.

There's a chance some people have not cleared their cache in two years
and still have tar based packages around. But forcing them to clear
their cache is not a big deal at all.
@Morganamilo
Copy link
Contributor Author

Annoyingly when searching for tar it also pulls in nextArg and target. So I couldn't really just do a find all. I was kind of relying on the CI to catch the unused for me.

@Morganamilo Morganamilo merged commit 0bd21ea into Jguer:master Nov 11, 2019
@bundi78
Copy link

bundi78 commented Jan 16, 2020

Hi,

sorry to be late, but for me, that feature was useful. I have to work behind an awkward proxy and --nogitclone was the only working solution to use AUR.

With 9.4.3, I have no (easy) access to AUR.

@mjruschak
Copy link

Oh boy... Any way you could add this back. Found this option useful for behind proxys. Much appreciated if we could get it back.

@mjruschak mjruschak mentioned this pull request Jan 27, 2020
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.

4 participants