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.

Sign up for GitHub

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

Jump to bottom

chore: Replaced apt with apt-get #804

Merged
merged 6 commits into from
Oct 24, 2024
Merged

chore: Replaced apt with apt-get #804

merged 6 commits into from
Oct 24, 2024

Conversation

prakrit55
Copy link
Contributor

Describe the changes in this pull request using active verbs such as Add, Remove, Replace ...

  • Replaced apt with apt-get

Closes #787

@prakrit55
Copy link
Contributor Author

@ashnamehrotra, can you check it once, if it is okay

@codecov Codecov
Copy link

codecov bot commented Oct 9, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 57.14286% with 9 lines in your changes missing coverage. Please review.

Project coverage is 46.42%. Comparing base (e5f901b) to head (8e45080).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
pkg/pkgmgr/dpkg.go 57.14% 9 Missing ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##             main     #804   +/-   ##
=======================================
  Coverage   46.42%   46.42%           
=======================================
  Files          18       18           
  Lines        2182     2182           
=======================================
  Hits         1013     1013           
  Misses       1111     1111           
  Partials       58       58

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@prakrit55 prakrit55 changed the title replaced apt with apt-get chore: Replaced apt with apt-get Oct 9, 2024
ashnamehrotra
ashnamehrotra reviewed Oct 9, 2024
View reviewed changes
pkg/pkgmgr/dpkg.go
} else {
// if updates is not specified, update all packages
installCmd = `sh -c "output=$(apt upgrade -y && apt clean -y && apt autoremove 2>&1); if [ $? -ne 0 ]; then echo "$output" >>error_log.txt; fi"`
installCmd = `sh -c "output=$(apt-get upgrade -y && apt-get clean -y && apt-get autoremove 2>&1); if [ $? -ne 0 ]; then echo "$output" >>error_log.txt; fi"`
Copy link
Contributor

Choose a reason for hiding this comment

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

it seems like this command is not downloading the packages, resulting in the error for the test cases with no report

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let me check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ashnamehrotra, could you check it once, and this too

Copy link
Contributor

Choose a reason for hiding this comment

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

it looks like results manifest file is not found, so the packages are not found/downloaded correctly. There is also a log output line 2: packages.txt: No such file or directory. Would you be able to run the apt-get commands manually in a container to see if they are working as expected?

sozercan
sozercan reviewed Oct 10, 2024
View reviewed changes
pkg/pkgmgr/dpkg.go Outdated
@@ -411,7 +411,7 @@ func (dm *dpkgManager) unpackAndMergeUpdates(ctx context.Context, updates unvers
while IFS=':' read -r package version; do
pkg_name=$(echo "$package" | sed 's/^"\(.*\)"$/\1/')
pkg_version=$(echo "$version" | sed 's/^"\(.*\)"$/\1/')
latest_version=$(apt show $pkg_name 2>/dev/null | awk -F ': ' '/Version:/{print $2}')
latest_version=$(apt-get show $pkg_name 2>/dev/null | awk -F ': ' '/Version:/{print $2}')
Copy link
Member

Choose a reason for hiding this comment

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

apt-get show is not a valid command

sozercan
sozercan reviewed Oct 10, 2024
View reviewed changes
website/docs/faq.md Outdated
@@ -43,7 +43,7 @@ Experimental: This feature might change without preserving backwards compatibili

:::

Copa does not support replacing the repositories in the package managers with alternatives. Images must already use the intended package repositories. For example, for debian, updating `/etc/apt/sources.list` from `http://archive.ubuntu.com/ubuntu/` to a mirror, such as `https://mirrors.wikimedia.org/ubuntu/`.
Copa does not support replacing the repositories in the package managers with alternatives. Images must already use the intended package repositories. For example, for debian, updating `/etc/apt-get/sources.list` from `http://archive.ubuntu.com/ubuntu/` to a mirror, such as `https://mirrors.wikimedia.org/ubuntu/`.
Copy link
Member

@sozercan sozercan Oct 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

there's no /etc/apt-get dir

sozercan
sozercan reviewed Oct 10, 2024
View reviewed changes
pkg/pkgmgr/dpkg.go Outdated
@@ -24,7 +24,7 @@ const (
dpkgLibPath = "/var/lib/dpkg"
dpkgStatusPath = dpkgLibPath + "/status"
dpkgStatusFolder = dpkgLibPath + "/status.d"
dpkgDownloadPath = "/var/cache/apt/archives"
dpkgDownloadPath = "/var/cache/apt-get/archives"
Copy link
Member

@sozercan sozercan Oct 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

/var/cache/apt-get is not a valid dir

please make sure these exist before find and replace

sozercan
sozercan requested changes Oct 10, 2024
View reviewed changes
Copy link
Member

@sozercan sozercan left a comment

Choose a reason for hiding this comment

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

see comments

@sozercan
Copy link
Member

@prakrit55 looks like there's a lint issue

@prakrit55
Copy link
Contributor Author

prakrit55 commented Oct 12, 2024 •
edited
Loading

@prakrit55 looks like there's a lint issue

there are some other instances of apt in the docs, let me cover those too

@prakrit55
replaced apt with apt-get
fdd6f4c 
Signed-off-by: Griffin <prakritimandal611@gmail.com>
@prakrit55
fix
373e97a 
Signed-off-by: Griffin <prakritimandal611@gmail.com>
@prakrit55
fix
e05331d 
Signed-off-by: Griffin <prakritimandal611@gmail.com>
@prakrit55
go-lint checks
01e4f01 
Signed-off-by: Griffin <prakritimandal611@gmail.com>
sozercan
sozercan approved these changes Oct 24, 2024
View reviewed changes
Copy link
Member

@sozercan sozercan left a comment

Choose a reason for hiding this comment

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

Thank you! LGTM

@sozercan sozercan merged commit 6dd5b5e into project-copacetic:main Oct 24, 2024
25 checks passed
shubhamsugara22 pushed a commit to shubhamsugara22/copacetic that referenced this pull request Nov 13, 2024
@prakrit55 @sozercan @shubhamsugara22
chore: Replaced apt with apt-get (project-copacetic#804)
8ac4dc3 
Signed-off-by: Griffin <prakritimandal611@gmail.com>
Co-authored-by: Sertaç Özercan <852750+sozercan@users.noreply.github.com>
Signed-off-by: shubhamsugara22 <shubhamsugara22@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ashnamehrotra ashnamehrotra ashnamehrotra left review comments

@sozercan sozercan sozercan approved these changes

@jeremyrickard jeremyrickard Awaiting requested review from jeremyrickard jeremyrickard is a code owner

Assignees
No one assigned
Labels
None yet
Projects
Copacetic Workboard
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[REQ] Use apt-get instead of apt
3 participants
@prakrit55 @sozercan @ashnamehrotra