ungoogled-software / ungoogled-chromium-fedora

RPM build for ungoogled-chromium
BSD 3-Clause "New" or "Revised" License
33 stars 6 forks source link

Implement OBS Upload workflow #4

Closed ghost closed 3 years ago

ghost commented 3 years ago

I have some questions in addition to inline comments:

* Can depot_tools.git-master.tar.gz be fetched from `commit=$(grep 'depot_tools.git' src/DEPS | cut -d\' -f8)` instead of uploading each time?

* Does `_services` check checksums?

* How do I do development build now without pushing to this repo?
ghost commented 3 years ago

I'm contemplating getting rid of _constraints in order to put that stuff in the main project config.

wchen342 commented 3 years ago

My script just extracts any files that include a http or https URL and translates it to something that the _service file can use.

Ah I see. I need to update the link then. From what I read in the script it does not expand variables in spec file, so the link needs to be static?

_service doesn't check checksums unless you instruct it to. Doesn't RPM do that already though with the sources file I saw in the repository?

No, that file I just kept from upstream (Fedora build server checks it). You need to let _services check it.

getting rid of _constraints in order to put that stuff in the main project config

What's the differences? If constraints are all the same then using project config should be the same?

ghost commented 3 years ago

No, it does expand it. rpmspec also expands variables in URLs. The shell is using the expanded version of it.

As for _service that won't be easy to integrate as I need to get the checksums from somewhere. It may be best if I break up this work into a set of smaller commits that are easier to integrate.

The different is _constraints is per package instead of per project. It's a holdover from when OBS was only used for Debian stuff. I didn't even know it was possible to set it from the project config.

wchen342 commented 3 years ago

The shell is using the expanded version of it.

Ok then it shall be easy to update.

As for _service that won't be easy to integrate as I need to get the checksums from somewhere.

Can't you compare against the contents of sources? I update the file every commit so it is up to date.

ghost commented 3 years ago

That would probably work. I'll need to revamp my script a bit though as I wasn't planning on needing to do this. Arch repository does checksumming as part of its normal build process. All the debian based ones do so as well.

ghost commented 3 years ago

@wchen342 See #5 for a quick set of auxiliary fixes I found during my work.

wchen342 commented 3 years ago

@wchen342 See #5 for a quick set of auxiliary fixes I found during my work.

Merged.

ghost commented 3 years ago

Ok. Thanks. I'll see if i have anything else that's unrelated to OBS at present.

ghost commented 3 years ago

Ok. I seem to have found a way to make it work but I am going to have to add a new checksum for the unmodified tarball.

ghost commented 3 years ago

@wchen342 New problem. It appears that OBS doesn't support sha512 in this context. The strongest hash they support is sha256. Is it a problem if we switch to using that?

wchen342 commented 3 years ago

It should be fine. Just all checksums need to be updated then.

ghost commented 3 years ago

@wchen342 See #6 for the new sources.

ghost commented 3 years ago

@wchen342 As for the depot files source, any ideas how you want to solve it? From what I can tell I can't take it from the spec file like I do everything else. OBS allows renaming downloaded files but RPM doesn't appear to support that. So it looks like the only optional might be some form of hard-coded special case.

wchen342 commented 3 years ago

I updated the link, can it work now? It doesn't need to be renamed actually.

ghost commented 3 years ago

I'll try it.

ghost commented 3 years ago

Ugh. verify_file is failing and I can't figure out why. But it appears to be choking on the new name for the depot files tarball.

wchen342 commented 3 years ago

The filename is expanded using a global variable, is that the reason?

ghost commented 3 years ago

Doubtful as we have other source working that also use global variables.

ghost commented 3 years ago

I'll see what happens next. I've tried commenting out the verify_file just for the one it complains about.

ghost commented 3 years ago

Ok. I can't seem to get it to work because of the file name. Strange. In any case I do think there's one other option available. We could verify the sources files right before building.

wchen342 commented 3 years ago

What does the error say?

The tar file is expanded at the beginning of %prep so you need to verify before that.

ghost commented 3 years ago

For what it's worth, this is the error:

Files could not be expanded: service error: Running /usr/lib/obs/service//verify_file --file _service:download_url:428143ee24c5f084c8dfb38cd17f07b4f7ba9bf7.tar.gz --verifier sha256 --checksum dfae74fd6b3d982583fee8acfeec64d8994865dd3550f642be4b1d0d4998dd5d --outdir /var/cache/obs/2H40ZdKtXU0I/out less info

service verify_file failed:
time="2021-04-15T16:25:29Z" level=warning msg="Path \"/etc/SUSEConnect\" from \"/etc/containers/mounts.conf\" doesn't exist, skipping"
time="2021-04-15T16:25:29Z" level=warning msg="Path \"/etc/zypp/credentials.d/SCCcredentials\" from \"/etc/containers/mounts.conf\" doesn't exist, skipping"
Running /usr/lib/obs/service//verify_file --file _service:download_url:428143ee24c5f084c8dfb38cd17f07b4f7ba9bf7.tar.gz --verifier sha256 --checksum dfae74fd6b3d982583fee8acfeec64d8994865dd3550f642be4b1d0d4998dd5d --outdir /var/cache/obs/2H40ZdKtXU0I/out
wchen342 commented 3 years ago

The filename is wrong somehow. It should be depot_tools.git-428143ee24c5f084c8dfb38cd17f07b4f7ba9bf7.tar.gz.

ghost commented 3 years ago

You mean to tell me it uses content disposition?

wchen342 commented 3 years ago

Probably, if you download the link from browser it has a different filename.

ghost commented 3 years ago

Right, content disposition is a nightmare for a number of package setups. They work around it by allowing renaming of the files locally. I don't know if it's related to this but I'll see what I can do.

ghost commented 3 years ago

Ok. I've found the problem. The remote web service is creating the output on demand in a manner that produces different files so there's no way to apply a checksum to it.

ghost commented 3 years ago

Try downloading it multiple times, from here or so: https://chromium.googlesource.com/chromium/tools/depot_tools.git/+archive/428143ee24c5f084c8dfb38cd17f07b4f7ba9bf7.tar.gz

ghost commented 3 years ago

You should get a different file each time. So how do you want to solve this?

wchen342 commented 3 years ago

Can you create the file using service by running git?

ghost commented 3 years ago

I can create an archive from SCM. Do you require tar or can it be cpio based?

wchen342 commented 3 years ago

RPM can only extract tar, zip, gzip, bzip2, pack, compress, and lzh.

ghost commented 3 years ago

Ok. I consolidated everything into a single commit. I'll see if there's anything else I need to do.

ghost commented 3 years ago

Just one more thing that I can tell. Right now I'm waiting to see if the OBS stuff builds successfully as I still need to tweak the project config until I figure out what will work for Fedora.

ghost commented 3 years ago

I also noticed a small issue with what I was doing. As it stands I copy all the regular files from the git repository which may include files that are not in the specfile as a local file. Do you want me to limit it files only in the specfile?

ghost commented 3 years ago

Ok... I just need to figure why the builds fail to find their depot tools stuff now.

wchen342 commented 3 years ago

may include files that are not in the specfile as a local file

You mean the files like README and LICENSE? That should be fine.

ghost commented 3 years ago

Well it was grabbing everything from the directory committed or not. I decided to rewrite my parsing code so it'll skip over any other files that are present. I figure if we want to include the extras we should hard-code their inclusion into the script.

ghost commented 3 years ago

The script should be nearly done. I think the only thing left to confirm is that the whole thing builds.

ghost commented 3 years ago

Ok. I found the issue with depot_tools. It extracts the package to ../depot_tools but it contains an extra directory. So it's actually ../depot_tools/depot_tools-$version. Any ideas how you want to address this?

wchen342 commented 3 years ago

Can it just ignore the inner directory? It's better to keep paths consistent I think.

ghost commented 3 years ago

Maybe, where's the documentation for this %setup directive? I get the feeling it might need to be tweaked.

wchen342 commented 3 years ago

Currently it create a directory and extract the compressed file in it, so I think you need to cd into the git folder before compress.

About %setup see here: https://docs.fedoraproject.org/en-US/Fedora_Draft_Documentation/0.1/html-single/RPM_Guide/index.html, the Preparing for the build section.

ghost commented 3 years ago

I suspect this is an issue because the tar_scm service puts all the files in a directory whereas the old tarball had everything top-level with no prefix directory.

ghost commented 3 years ago

I think I found a work-around. I'll change how it's extracted and use a symlink to reference the actual directory. That way nothing else needs to be changed.

wchen342 commented 3 years ago

Yeah it can work, I used a similar trick when building Android repo too. Let me know when the build succeeded.

ghost commented 3 years ago

I have integrated the fix. Right now I just need to see that it builds.

ghost commented 3 years ago

One thing I've noticed is your builds produce a lot of noisy warnings which makes it harder to see when something more serious comes up. Debian builds don't show as many warnings. Perhaps you can try disabling superfluous warnings or so. This is not my packaging so I'm not going to try to fix something like that.

For example, here's a build log for debian sid: https://build.opensuse.org/package/live_build_log/home:ungoogled_chromium/ungoogled-chromium-sid/Debian_Sid/x86_64

Notice how much cleaner the log is generally speaking. It's a lot easier to track the build progress.

wchen342 commented 3 years ago

It's probably because of https://github.com/ungoogled-software/ungoogled-chromium-fedora/blob/57002b9d2730464eec1be08ad2950aef68eab25a/ungoogled-chromium.spec#L26, I shall add a flag to switch that off later.