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

feat: add new slice definitions for 'git' and dependancies #282

Merged
merged 34 commits into from
Nov 25, 2024

Conversation

clay-lake
Copy link

@clay-lake clay-lake commented Jul 15, 2024

Proposed changes

Add slices for git and dependency libcurl3t64-gnutls.

Related issues/PRs

Testing

Rudimentary smoke testing was provided by the script below. The bins slice successfully executes basic git operations, except for cloning a repository over http, which requires the core slice. Cloning over ssh has not yet been tested.

set -e

function echo_status(){
        $@ > /dev/null
        echo  $@ " " $? 
}

cd figlet-local
echo_status git status
echo_status git log
echo_status git config --global user.email "NA"
echo_status git config --global user.name "NA"
echo_status git checkout -b test
echo //new comment > getopt.c
echo_status git commit -a -m test_commit
echo_status git checkout master
echo_status git branch
echo_status git merge test

echo_status cd ~

echo_status mkdir figlet-http
echo_status git clone https://github.com/cmatsuoka/figlet.git figlet-http

echo TESTS PASS

All testing was performed on a chiseled (ubuntu-24.04) container containing the following slices.

base-files_base 
base-files_release-info 
busybox_bins 
git_bins # or git_core
ca-certificates_data

Checklist

Copy link

github-actions bot commented Jul 15, 2024

Diff of dependencies:

slices/git.yaml
@@ -1,8 +1,5 @@
-git-man
 libc6
 libcurl3t64-gnutls
-liberror-perl
 libexpat1
 libpcre2-8-0
-perl
 zlib1g

Copy link
Member

@rebornplusplus rebornplusplus left a comment

Choose a reason for hiding this comment

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

Nice! Note that the Lint is failing.

slices/git.yaml Outdated Show resolved Hide resolved
slices/git.yaml Outdated Show resolved Hide resolved
slices/git.yaml Outdated Show resolved Hide resolved
Copy link
Member

@rebornplusplus rebornplusplus left a comment

Choose a reason for hiding this comment

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

Looking good! I just left two comments. let me know what you think!

slices/git.yaml Outdated Show resolved Hide resolved
slices/git.yaml Outdated Show resolved Hide resolved
Copy link
Member

@rebornplusplus rebornplusplus left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you!

Copy link
Collaborator

@cjdcordeiro cjdcordeiro left a comment

Choose a reason for hiding this comment

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

Great! Thanks.

Are you sure you don't need liberror-perl and perl? If so, then there's just one small comment and it should be good to go

slices/git.yaml Outdated Show resolved Hide resolved
Copy link
Collaborator

@cjdcordeiro cjdcordeiro left a comment

Choose a reason for hiding this comment

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

let's add some tests too

@clay-lake clay-lake marked this pull request as draft October 2, 2024 20:07
@lengau lengau mentioned this pull request Oct 20, 2024
3 tasks
@rebornplusplus rebornplusplus added the Priority Look at me first label Oct 31, 2024
Copy link
Author

@clay-lake clay-lake left a comment

Choose a reason for hiding this comment

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

Requested changes from Oct 24 Sprint

slices/git.yaml Show resolved Hide resolved
slices/git.yaml Show resolved Hide resolved
tests/spread/integration/git/task.yaml Show resolved Hide resolved
@clay-lake clay-lake marked this pull request as ready for review November 8, 2024 10:18
@clay-lake
Copy link
Author

clay-lake commented Nov 15, 2024

@lengau Thanks! Looks good. I replaced my test with yours, and added some of the changes I was working on since Cris' review.
I also added support for some commands @dariuszd21 requested:

  • git branch --remotes --contains
  • git rev-list -n 1
  • git fetch

Copy link

@lengau lengau left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! One question about the pyftpd, but otherwise I'm ready to do a final sync to my branch as soon as we hear a go.

slices/python3-pyftpdlib.yaml Show resolved Hide resolved
Copy link
Collaborator

@cjdcordeiro cjdcordeiro left a comment

Choose a reason for hiding this comment

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

The python test also needs fixing it seems

slices/git.yaml Outdated Show resolved Hide resolved
@clay-lake
Copy link
Author

The python tests are working now. It was a mismatch between python versions.

Copy link
Member

@rebornplusplus rebornplusplus left a comment

Choose a reason for hiding this comment

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

Looks nice and small, thanks! I left a few comments below.

slices/git.yaml Show resolved Hide resolved
slices/git.yaml Outdated Show resolved Hide resolved
slices/git.yaml Outdated Show resolved Hide resolved
slices/python3-pyftpdlib.yaml Outdated Show resolved Hide resolved
tests/spread/integration/git/task.yaml Show resolved Hide resolved
tests/spread/integration/git/task.yaml Show resolved Hide resolved
Copy link
Collaborator

@cjdcordeiro cjdcordeiro left a comment

Choose a reason for hiding this comment

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

lgtm thanks

@cjdcordeiro
Copy link
Collaborator

@clay-lake can you pls raise these changes to 24.10 too?

Copy link
Member

@rebornplusplus rebornplusplus left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you very much!

@rebornplusplus rebornplusplus merged commit 5143451 into canonical:ubuntu-24.04 Nov 25, 2024
14 checks passed
clay-lake added a commit to clay-lake/chisel-releases that referenced this pull request Nov 25, 2024
@clay-lake
Copy link
Author

Will do! @cjdcordeiro #396

@lengau Merge is complete :)
There are few more changes for you to pull before you merge your PR.

lengau added a commit to lengau/chisel-releases that referenced this pull request Dec 12, 2024
Synchronises this branch to the current state of the in-progress
Ubuntu 24.04 slice PR.

This is incomplete because there are currently questions about the tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority Look at me first
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants