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

Submodules may contain other submodules. Do process and add those to the source tar. #457

Merged
merged 1 commit into from
May 16, 2023

Conversation

arcivanov
Copy link
Contributor

fixes #456

@FrostyX
Copy link
Member

FrostyX commented Apr 28, 2023

Thank you for the PR @arcivanov,
do you have some project on which I can try it?

@arcivanov
Copy link
Contributor Author

Thank you for the PR @arcivanov, do you have some project on which I can try it?

Yes. https://github.com/karellen/karellen-sysbox
The builder in https://github.com/karellen/karellen-sysbox/blob/master/.tito/tito.props is what I'm donating back.
The only change is formatting to satisfy your linting requirements.

@arcivanov
Copy link
Contributor Author

@FrostyX yield from doesn't seem to be supported in epel7 builds. What versions of Python have to be supported?

@arcivanov arcivanov force-pushed the issue_456 branch 2 times, most recently from f9f72fb to 5b21da9 Compare April 28, 2023 21:00
@arcivanov
Copy link
Contributor Author

@FrostyX fixed

@FrostyX
Copy link
Member

FrostyX commented May 1, 2023

Thank you for the CI fixes and so on :-)

The PR works for me. I only found one issue, when I did

git clone https://github.com/karellen/karellen-sysbox.git
cd karellen-sysbox/
tito build --srpm

And forgot to pull the submodules first. It fails with a traceback:

[jkadlcik@zeratul karellen-sysbox]$ tito build --srpm 
Creating output directory: /tmp/tito
Checking for tag [karellen-sysbox-0.6.1.17-3] in git repo [https://github.com/karellen/karellen-sysbox.git]
Building package [karellen-sysbox-0.6.1.17-3]
ERROR: Error running command: git archive --format=tar --prefix=karellen-sysbox-0.6.1.17/sysbox/ f1cc75cfe5f13cc7f86fb3a0e16b2ae19748c064: --output=/tmp/tito/rpmbuild-karellen-sysboxekzstf_9/SOURCES/karellen-sysbox-0.6.1.17.tar.initial.sysbox

Status code: 128

Command output: fatal: not a valid object name: f1cc75cfe5f13cc7f86fb3a0e16b2ae19748c064:

Traceback (most recent call last):
  File "/usr/bin/tito", line 33, in <module>
    sys.exit(load_entry_point('tito==0.6.22', 'console_scripts', 'tito')())
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/tito/cli.py", line 910, in main
    CLI().main(sys.argv[1:])
  File "/usr/lib/python3.11/site-packages/tito/cli.py", line 209, in main
    return module.main(argv)
           ^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/tito/cli.py", line 392, in main
    return builder.run(self.options)
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/tito/builder/main.py", line 162, in run
    self.srpm()
  File "/usr/lib/python3.11/site-packages/tito/builder/main.py", line 221, in srpm
    self.tgz()
  File "/usr/lib/python3.11/site-packages/tito/builder/main.py", line 573, in tgz
    self._setup_sources()
  File "/tmp/karellen-sysbox/.tito/libs/karellen/tito/submodule_aware_builder.py", line 60, in _setup_sources
    self.create_tgz(
  File "/tmp/karellen-sysbox/.tito/libs/karellen/tito/submodule_aware_builder.py", line 185, in create_tgz
    for submodule_tar_file in self._submodule_archives(
  File "/tmp/karellen-sysbox/.tito/libs/karellen/tito/submodule_aware_builder.py", line 136, in _submodule_archives
    self.run_git_archive(
  File "/tmp/karellen-sysbox/.tito/libs/karellen/tito/submodule_aware_builder.py", line 101, in run_git_archive
    run_command(git_archive_cmd)
  File "/usr/lib/python3.11/site-packages/tito/common.py", line 305, in run_command
    raise RunCommandException(command, status, output)
tito.exception.RunCommandException: Error running command: git archive --format=tar --prefix=karellen-sysbox-0.6.1.17/sysbox/ f1cc75cfe5f13cc7f86fb3a0e16b2ae19748c064: --output=/tmp/tito/rpmbuild-karellen-sysboxekzstf_9/SOURCES/karellen-sysbox-0.6.1.17.tar.initial.sysbox

It probably failed with a traceback even before your PR, but since you are touching that part of the code, could you please catch the exception and print some reasonable error message? Ideally, showing users the command to pull the submodules?

@FrostyX
Copy link
Member

FrostyX commented May 1, 2023

Hello @t0fik, you recently did some coding on SubmoduleAwareBuilder. Can you please take a look if this PR works for you?

@FrostyX
Copy link
Member

FrostyX commented May 1, 2023

Also @areese, do you want to take a look, please?

@arcivanov
Copy link
Contributor Author

It probably failed with a traceback even before your PR, but since you are touching that part of the code, could you please catch the exception and print some reasonable error message? Ideally, showing users the command to pull the submodules?

I had the exactly same issue a couple of times 😄
I could do is check if the submodule directory contains .git subdir and if not raise an error asking whether they forgot to recursively pull submodules. Or I could try pulling the submodule recursively and try again.

Please let me know what you think.

BTW, this is running on Fedora COPR already.
https://copr.fedorainfracloud.org/coprs/karellen/karellen-sysbox/

submodule_commit,
submodule_tar_file,
submodule,
)
Copy link
Member

Choose a reason for hiding this comment

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

I think, maybe you can run the self.run_git_archive(...) inside of a try-except block and if you find something like "not a valid object name" in the exception, you can create some user-friendly message and do raise TitoException(msg).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, but the failure could be something else like out of disk space etc. to blindly try-catch or I'd have to parse the error.
There is no situation where the submodule is checked out recursively but there is no .git sub-directory though, i.e. it's a sure way to check if it was correctly pulled.

Copy link
Member

Choose a reason for hiding this comment

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

You are right :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does this work for you?

$ tito build --srpm --debug
Creating output directory: /tmp/tito
Added lib dir to PYTHONPATH: /home/arcivanov/Documents/src/karellen/karellen-sysbox/.tito/libs
---- Builder class is None
Importing karellen.tito.SubmoduleAwareBuilder
Using builder class: <class 'karellen.tito.submodule_aware_builder.SubmoduleAwareBuilder'>
Building in temp dir: /tmp/tito/rpmbuild-karellen-sysbox2l092siv
Getting latest package info from: /home/arcivanov/Documents/src/karellen/karellen-sysbox/.tito/packages/karellen-sysbox
Command: awk '{ print $1 ; exit }' /home/arcivanov/Documents/src/karellen/karellen-sysbox/.tito/packages/karellen-sysbox
Status code: 0
Command output: 0.6.1.17-3

Command: git ls-remote ./. --tag karellen-sysbox-0.6.1.17-3 | awk '{ print $1 ; exit }'
Status code: 0
Command output: 559a13650b1241d37cff335b5b2df0d36d3d2d39

Local tag SHA1: 559a13650b1241d37cff335b5b2df0d36d3d2d39
Command: git config remote.origin.url
Status code: 0
Command output: ssh://git@github.com/karellen/karellen-sysbox

Command: git config remote.origin.url
Status code: 0
Command output: ssh://git@github.com/karellen/karellen-sysbox

Checking for tag [karellen-sysbox-0.6.1.17-3] in git repo [ssh://git@github.com/karellen/karellen-sysbox]
Command: git ls-remote ssh://git@github.com/karellen/karellen-sysbox --tag karellen-sysbox-0.6.1.17-3 | awk '{ print $1 ; exit }'
Status code: 0
Command output: 559a13650b1241d37cff335b5b2df0d36d3d2d39

Remote tag SHA1: 559a13650b1241d37cff335b5b2df0d36d3d2d39
Command: git ls-remote ./. --tag karellen-sysbox-0.6.1.17-3 | awk '{ print $1 ; exit }'
Status code: 0
Command output: 559a13650b1241d37cff335b5b2df0d36d3d2d39

Command: git rev-list --max-count=1 559a13650b1241d37cff335b5b2df0d36d3d2d39
Status code: 0
Command output: a270b8ed4363962ec55efd2d41788fc2fc30dad4

Got package metadata: ['0.6.1.17-3', './']
Building package [karellen-sysbox-0.6.1.17-3]
Creating karellen-sysbox-0.6.1.17.tar.gz from git tag: a270b8ed4363962ec55efd2d41788fc2fc30dad4...
Command: git rev-list --timestamp --max-count=1 a270b8ed4363962ec55efd2d41788fc2fc30dad4 | awk '{print $1}'
Status code: 0
Command output: 1682902338

Command: git archive --format=tar --prefix=karellen-sysbox-0.6.1.17/ a270b8ed4363962ec55efd2d41788fc2fc30dad4: --output=/tmp/tito/rpmbuild-karellen-sysbox2l092siv/SOURCES/karellen-sysbox-0.6.1.17.tar.initial
Status code: 0
Command output: 

Command: git config --file .gitmodules --get-regexp path
Status code: 0
Command output: submodule.sysbox.path sysbox

Cleaning up /tmp/tito/rpmbuild-karellen-sysbox2l092siv
ERROR: '/home/arcivanov/Documents/src/karellen/karellen-sysbox/sysbox/.git' path does not contain '.git' directory. Have you cloned the repository recursively?

Copy link
Member

Choose a reason for hiding this comment

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

Thank you!

...karellen/karellen-sysbox/sysbox/.git' path does not contain '.git'

I am nitpicking: This is a bit confusing, I would drop the .git from the path.

Have you cloned the repository recursively?

Maybe suggesting the git submodule update --init --recursive command so that we don't have to search for it on stack overflow? 😄

But at this point, this PR has +1 from me anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nevermind. I have to go by the .git file, not .git directory. Fixing now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All good, the check is now that there is a file .git (directory or file) and that is checked.

The build passed no problem: https://copr.fedorainfracloud.org/coprs/karellen/karellen-sysbox/build/5865413/

Copy link
Member

Choose a reason for hiding this comment

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

Perfect, thank you very much @arcivanov.
Works for me, and the error message is now super helpful.

I'll leave the PR open for a couple of days so that anybody else has enough time to review if they want to. Then I'll merge on Monday.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ping 😆

Copy link
Member

Choose a reason for hiding this comment

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

Oh, it's two weeks already? Sorry about that.

@arcivanov arcivanov force-pushed the issue_456 branch 2 times, most recently from a3d9b72 to f9efca7 Compare May 1, 2023 22:15
@FrostyX FrostyX merged commit 835309b into rpm-software-management:master May 16, 2023
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.

Submodule_aware_builder (SAB) does not package submodules recursively
2 participants