teemtee / tmt

Test Management Tool
MIT License
76 stars 112 forks source link

Simplify packit custom create archive command #249

Closed psss closed 3 years ago

psss commented 4 years ago

@lachmanfrantisek, is there anything wrong with the config?

The create-archive action did not output a path to the generated archive. Please make sure that you have valid path in the single line of the output.

The full path to the archive is printed as the last line:

> make tarball
...
rst2man /home/psss/git/tmt/tmp/man.rst > /home/psss/git/tmt/tmp/tmt-0.16/tmt.1
cd /home/psss/git/tmt/tmp && tar cfz SOURCES/tmt-0.16.tar.gz tmt-0.16
/home/psss/git/tmt/tmp/SOURCES/tmt-0.16.tar.gz
lachmanfrantisek commented 4 years ago

It works nicely locally. So there is some problem with the service. I'll create an issue.

Sorry for this occurring problem.

lachmanfrantisek commented 4 years ago

Potential problem:

(You can try using relative paths, but I still create an issue for this.)

psss commented 4 years ago

@lachmanfrantisek, thanks for looking into this. The relative path does not work:

error: File ./tmt-0.16.tar.gz: No such file or directory

I've tried both tmp/SOURCES/... and ./tmp/SOURCES/.... Seems the only currently working way is to move the created archive to the top directory.

lachmanfrantisek commented 4 years ago

Thank you for testing all variants. We need to fix that on our side. There might a problem with the subdirectory.

psss commented 4 years ago

@lachmanfrantisek, thanks for looking into this. Let me know when I can test it again.

psss commented 3 years ago

/packit build

psss commented 3 years ago

@lachmanfrantisek, has there been any progress to make this working?

lachmanfrantisek commented 3 years ago

@lachmanfrantisek, has there been any progress to make this working?

Unfortunately no, sorry. But @TomasTomecek is now working on the shell expansion for commands and is having some problems that can be related to this.

TomasTomecek commented 3 years ago

/packit build

TomasTomecek commented 3 years ago

It seems that the archive is in ./tmp/SOURCES/tmt-0.18.tar.gz and not ./tmt-0.18.tar.gz.

psss commented 3 years ago

@TomasTomecek, yes, that was the intention: To simplify Makefile so that moving the archive is not necessary. Providing the full/relative path to the archive in the output should be sufficient for Packit to find it.

TomasTomecek commented 3 years ago

@TomasTomecek, yes, that was the intention: To simplify Makefile so that moving the archive is not necessary. Providing the full/relative path to the archive in the output should be sufficient for Packit to find it.

So you're saying that the build should pass with the current changes and the fact the SRPM can't be created is wrong?

psss commented 3 years ago

@TomasTomecek, that's what I've understood from our discussion with @lachmanfrantisek: That if the custom archive command prints out a valid path the to archive it should just work. Which makes sense to me. Otherwise users are forced to move the archive to proper location. Fixing this in one place (packit) seems to me to be a better option than duplicating this unnecessary action again and again in individual Makefiles.

lachmanfrantisek commented 3 years ago

@TomasTomecek We are creating the symlink, but it does not work for some reason in the service:

Using user-defined script for ActionName.create_archive: [['make', 'tarball']]
Running command: make tarball
06:33:42.377 base_git.py       DEBUG  Action command output: ['rm -rf /sand...
Created archive: /sandcastle/tmp/SOURCES/tmt-0.18.tar.gz
Linking to the specfile directory: /sandcastle/tmt-0.18.tar.gz
:
Running command: rpmbuild -bs --define _sourcedir . --define _srcdir . --define _specdir . --define _srcrpmdir . --define _topdir . --define _builddir . --define _rpmdir . --define _buildrootdir . tmt.spec

Message: An unexpected error occurred when creating the SRPM: Command failed (rc=1, reason={"metadata":{},"status":"Failure","message":"command terminated with non-zero exit code: Error executing in Docker Container: 1","reason":"NonZeroExitCode","details":{"causes":[{"reason":"ExitCode","message":"1"}]}})
error: File ./tmt-0.18.tar.gz: No such file or directory
TomasTomecek commented 3 years ago

I suspect the symlinks are not being propagated well b/w sandbox and worker.

Anyway, I'm really confused here. I can see the tarball path being printed:

cd /sandcastle/docker-io-usercont-sandcastle-prod-20200729-063317269798/tmp && tar cfz SOURCES/tmt-0.18.tar.gz tmt-0.18
./tmp/SOURCES/tmt-0.18.tar.gz\n']

So how come that packit goes for error: File ./tmt-0.18.tar.gz: No such file or directory and not for ./tmp/SOURCES/....

We should probably add some debug prints in packit codebase whether it's able to find the archive. We'd also know if the symlinking works.

mfocko commented 3 years ago

I've looked around it a bit; enforced same handling of paths as in service.

So how come that packit goes for error: File ./tmt-0.18.tar.gz: No such file or directory and not for ./tmp/SOURCES/....

@TomasTomecek That is because the build command is called with every directory set as . (when ran locally produces absolute path, when ran in service uses .).

Tried to break it in multiple ways:

psss commented 3 years ago

Trying with @echo $(TMP)/SOURCES/$(PACKAGE).tar.gz to print the path.

psss commented 3 years ago

Seems this did not help either. The resulting path was:

/sandcastle/docker-io-usercont-sandcastle-prod-20200803-125030982828/tmp/SOURCES/tmt-0.20.tar.gz

But:

Message: Preparing of the upstream to the SRPM build failed:
The create-archive action did not output a path to the generated archive.
Please make sure that you have valid path in the single line of the output.
mfocko commented 3 years ago

Sorry for the confusion, packit requires relative path and the $(TMP) got expanded into absolute path.

I think it would be best to continue with ./tmp/SOURCES/$(PACKAGE).tar.gz, since it passed the file-existence check in packit and focus on the symlinking on our side (I have packit/packit#923 ready to see what's going on with the relative/absolute paths while linking and checking existence).

Also would you like to try stage version (more info: https://github.com/packit-service/packit-service/issues/649) of packit? It would help with catching problems early and speed up debugging of problems like this. :)

psss commented 3 years ago

Ok, I'll revert the change to use relative path. I've also enabled the staging instance. Let's see...

mfocko commented 3 years ago

Thanks. PR with extended logging is not merged yet, I'll trigger rebuild when it gets merged.

mfocko commented 3 years ago

/packit build

packit-as-a-service-stg[bot] commented 3 years ago

Only users with write or admin permissions to the repository can trigger Packit-as-a-Service

packit-as-a-service[bot] commented 3 years ago

Only users with write or admin permissions to the repository can trigger Packit-as-a-Service

lachmanfrantisek commented 3 years ago

/packit build

TomasTomecek commented 3 years ago

we should make @mfocko packit admin :)

it seems the change you did in packit is not deployed to stg yet, let me retrigger stg worker image build

mfocko commented 3 years ago

/packit build

packit-as-a-service[bot] commented 3 years ago

Only users with write or admin permissions to the repository can trigger Packit-as-a-Service

packit-as-a-service-stg[bot] commented 3 years ago

Only users with write or admin permissions to the repository can trigger Packit-as-a-Service

psss commented 3 years ago

/packit build

TomasTomecek commented 3 years ago

Wow, passed! are we good here? I just can't believe it that the fix was so "easy"

psss commented 3 years ago

Nice! Looks good :)

psss commented 3 years ago

@lachmanfrantisek, @TomasTomecek, @mfocko, works nice in production as well. Thanks for fixing this!

TomasTomecek commented 3 years ago

@psss thank you for your patience and working with us!