Gitian builder

[micaelmalta]

No description provided.

[micaelmalta]

Ping @patricklodder @AbcSxyZ 🚀

[AbcSxyZ]

A first review with some fix to do and details to discuss.

In my opinion, I wouldn't add many of those files and keep the following :

• gitian-build.sh
• 0001-Docker-apt-cacher.patch (temporary)
• dependencies.sh (at least his content)
• (No opinion about cirrus, so maybe)

And remove all other scripts about setups, licensing, readme.

To explain myself about setup script, all those scripts looks overkill, and I would be in favor to replace it by an appropriate documentation.
I understand the desire for automation, but it seems a lot of engineering to set up a couple of dependencies, who require few lines to copy-paste.

A documentation who can be structured like that :

Redirect to docker documentation
+ add an instruction for debian distros to add user in docker group

 + add a single line by os
# Debian based distributions
sudo apt [dep] ..
# Mac
brew [dep] ..
...

It will also reduce content of gitian-build.sh by removing setup instructions, the documentation about the feature.
It may allow us to keep a single script with his documentation. Do you think these scripts are worth it ?

Regarding documentation, I did not review it because I wanted to discuss this setup topic, but at least I think it should be moved under doc instead of contrib, to update gitian-building.md, and maybe some other file like https://github.com/dogecoin/dogecoin/blob/master/contrib/gitian-descriptors/README.md.

[AbcSxyZ]

As a reminder, we should create an issue to track evolution of the PR inside the devrandom repo. To remove this fix when the feature is released.

[micaelmalta]

The PR is already created: devrandom/gitian-builder#249

[AbcSxyZ]

I was more thinking about opening an issue on Dogecoin repository, as a reminder to fix and remove the patch when they include your PR.

I would wait a bit and maybe do this when this PR is merged, they may merge before.

[fmhc]

I am running into issues with following the README on a plain Ubuntu system. I have set up my GPG key on the system, do I need to do anything else? How is the naming of the GPG key affecting the command?

It builds everything without significant errors but the generated binaries are not in the dogecoin-binaries/1.14.4-dev/ folder

[AbcSxyZ]

@fmhc
I tried your command, and wasn't able to get binaries from my side too using your command. Seem not working with a build using a commit, but fine with a tag.
This work for me :

./gitian-build.sh --build signer 1.14.3

For the build from a commit, seem to work properly, but maybe binaries are not moved in the appropriate directory ? Wasn't able to find them on the host, I didn't investigate further in the container rn.

@micaelmalta Were you able to build from a commit or didn't try ?

Also, I have been waiting but did not get an answer :) What about my question on install scripts ? Would be great to discuss, to see your pov.

[micaelmalta]

I am currently working on a test descriptor to check it

[micaelmalta]

@fmhc @AbcSxyZ I fixed some issues here.

@AbcSxyZ I really think the gitian-builder should be an independent repo from Dogecoin Core ... If we do so, the init scripts make more sense IMO. I remove the init script and moved into a dedicated folder on my repo https://github.com/micaelmalta/gitian-builder-dogecoin

[AbcSxyZ]

And think it as a standalone script inside the repository ? A script you can just download or copy to run the build.

Thinking quickly about using the current repository, it may be a bit messy do it that way, you thought about it too ? You had "issue" with git right (can't find this discussion again) ? Not sure if we should explore something here.

Personally, I'm really mitigated about using another repository, do not feel it's really needed.

[fmhc]

I was able to successfully build from dogecoin/dogecoin and from my own repo with commit flag, working fine for me!

And think it as a standalone script inside the repository ? A script you can just download or copy to run the build.

Thinking quickly about using the current repository, it may be a bit messy do it that way, you thought about it too ? You had "issue" with git right (can't find this discussion again) ? Not sure if we should explore something here.

Personally, I'm really mitigated about using another repository, do not feel it's really needed.

I thougth having it as a seperate repository would be nice as a single starting point, but this solution also works and i think its a great achievement for the dogecoin-core repo to enable "modern" docker-based development with this. guess its a general question if things should all be in the core or if dogecoin development should spread out over more repositories in the dogecoin project.

In terms of documentation, a hint for = gpg key uid name could be useful to avoid someone spending time gathering that knowledge. I had a GPG key but only with my full name, not a short one, breaking the input argument and then i thought it was the key filename until i digged into the code and @michaelMALTA giving some hints on his repository, thanks for that 🚀

[AbcSxyZ]

its a general question if things should all be in the core or if dogecoin development should spread out over more repositories

I'm not against using other repos sometime, but not for Gitian from my pov. I wouldn't be in favor of using another repo for a single script, and I think all install scripts are not needed, it's for me a lot of engineering for a couple of dependencies.
Good docs would do the job.

I like automation, but I'm wondering if it's appropriate here :)

[micaelmalta]

In terms of documentation, a hint for = gpg key uid name could be useful to avoid someone spending time gathering that knowledge. I had a GPG key but only with my full name, not a short one, breaking the input argument and then i thought it was the key filename until i digged into the code and @michaelMALTA giving some hints on his repository, thanks for that 🚀

@fmhc I added some documentation for the GNUGPG UID 🚀

[micaelmalta]

@rnicoll @patricklodder @michilumin What do you think about this? Should we just have a single script in the Dogecoin core repo, or a separate repo?

[patricklodder]

Should we just have a single script in the Dogecoin core repo, or a separate repo?

Scripts that are written for Dogecoin Core and aid the build process should imho go in contrib, together with all the other things that do that (like the scripts you wrote here). Scripts that are maintained outside of Dogecoin Core and are technically an external dependency, should not be maintained inside this repository; example: ltc_scrypt that is required for testing only has an installer script but the code remains external.

This makes sure that the repo stays on-topic and we do not over-burden ourselves with code maintenance from things that can be perfectly managed upstream.

[micaelmalta]

Should we just have a single script in the Dogecoin core repo, or a separate repo?

Scripts that are written for Dogecoin Core and aid the build process should imho go in contrib, together with all the other things that do that (like the scripts you wrote here). Scripts that are maintained outside of Dogecoin Core and are technically an external dependency, should not be maintained inside this repository; example: ltc_scrypt that is required for testing only has an installer script but the code remains external.

This makes sure that the repo stays on-topic and we do not over-burden ourselves with code maintenance from things that can be perfectly managed upstream.

Thanks @patricklodder ! Should we then consider to move all the scripts from https://github.com/micaelmalta/gitian-builder-dogecoin including the automated init and the CI tests?

[patricklodder]

Should we then consider to move all the scripts from https://github.com/micaelmalta/gitian-builder-dogecoin including the automated init and the CI tests?

Yes, but be careful with secrets... What I'd (ultimately) love to do with those scripts is:

1. Make a GH Actions (since this is targeting 1.14) script outputting hashes, that triggers on changes to:
   • ./depends
   • ./contrib/gitian-descriptors
   • ./contrib/gitian-builder (this set of scripts)
2. Any automated initialization should never touch pgp secret keys, so without those, it'd be fine to include. There's a section about external signing in the gitian doc and that should imho be the best practice anyway as this allows for keeping secrets in a protected environment while allowing a build machine to just connect freely and download things. (i.e. from a security p.o.v. there are different requirements to secure pgp secrets vs the gitian builder itself.)

[fmhc]

I think dogecoin should evolve and have a doge core repo for real core reference stuff, as @patricklodder said, and room for some supported / community approved repos in the dogecoin project domain. When will we have 20 signers for a release? We must make the dev community more integrative imho

[patricklodder]

Ran this a couple of times. Besides in-line comments, could you please revert the name change to the signing descriptors? Other than that, works neatly.

[patricklodder]

I think this should be:

1. A separate .patch file because this is a separate issue (and may or may not be fixed upstream regardless of the apt cache patch)
2. Conditional to only trigger on trusty because it's simply a fix for ubuntu depreciating the repository but not updating the docker image (see what I did for that earlier this year when we were building 1.14.3 at patricklodder/gitian-builder@de43391). Another image could have a fully functional esm source.

[patricklodder]

why change to host network? (also below in libexec/start-target)

[patricklodder]

MIT License is already listed on the top-level. No need to add it here again. Suggest to remove this file

[patricklodder]

This feels kind of redundant.

[AbcSxyZ]

What about instructions in https://github.com/dogecoin/dogecoin/blob/master/doc/gitian-building.md ?

Contrib folders have README but there is also some gitian instruction in doc. Those instructions in doc are outdated and do not work anymore. Where gitian instructions should go to avoid duplicate information, following DRY ?

Keep the contrib/gitian-builder README and add a link to it from the doc, maybe near a release process explanation ? And removing https://github.com/dogecoin/dogecoin/tree/master/doc/gitian-building folder with pictures + https://github.com/dogecoin/dogecoin/blob/master/doc/gitian-building.md ?

[patricklodder]

I'm okay with the readme, just the branding feels redundant.

[AbcSxyZ]

Where the gitian instructions should go ? Docs or contrib ?

[patricklodder]

Both. This readme should describe this script. The document in doc/ should describe the generic process regardless of this script but can mention and link it.

[patricklodder]

This has some implications towards how the pgp key for signing can be stored. Can we make this less intrusive towards security by doing the externally signed flavor?

[AbcSxyZ]

Are those credit useful ? Can reduce file content.

Do you mind @patricklodder ?

[patricklodder]

Yeah since the top level is already MIT, I would only add specific license info if there is a need to use a different license than MIT.

[AbcSxyZ]

I was speaking about the following credits :

## OTHER CREDITS

https://github.com/patricklodder

https://gist.github.com/patricklodder/88d6c4e3406db840963f85d95ceb44fe

I don't understand what you said about the license, why are you speaking about a different type of license ? No specific ressources in the folder is using another license right ? Couldn't we remove license info too ?

[patricklodder]

Sorry, my bad. Yeah credits can go, license can go.

[AbcSxyZ]

Do you think it's really useful to add this error ? It's more concerning some docker trouble shoot with rights, something that may be outside the scope here.

We may expect they will google their mistake, I think having documentation to anticipate potential error (at least those who do not concern dogecoin core) may add some unnecessary content.

Imagine we include also the #2337 with a Docker implementation to run nodes, should we add this info there also ?

[AbcSxyZ]

Using a fresh install, patch fail if git email is not configured using something like git config --global user.email "you@example.com".

[AbcSxyZ]

Don't know what did you do with all of your dependencies scripts, info were probably there, but looks like some are missing now.

Need ruby to run gbuild. Is it missing some prerequisites from devrandom/gitian-builder, maybe only ruby needed here ?

[AbcSxyZ]

I will try to improve the documentation for this topic in the following days, I'm going back into this PR.

@patricklodder @rnicoll do you think there is still any relevant information from the current gitian build guide ?
Wasn't working at all for me, lot of outdated content/versions, probably you use old environment on releases. For now, could we assume use docker as the default (and lonely) supported method to perform gitian ?
Delete all the current gitian docs and replace it with information related to this PR.

[patricklodder]

For now, could we assume use docker as the default (and lonely) supported method to perform gitian ?

No. Per the compile-time error in depends under gcc 7 that we found in #2501 by comparing docker with lxc output, we absolutely need more than just docker and we need to perform amendments to this script after we merge something.

[AbcSxyZ]

I just suggest some improvement, I think the process_file can be simplified in multiple ways, to finally keep a single function. Remove some extra content (like check_file echos), and make it easier to read by reducing the number of command substitution.

It's to keep it here as reminder, I may suggest a PR soon, but by trying to simplify the code of this file I ended up with :

function process_file() {
    local filename=$(basename $1)
    
    # Download file if not available
    if [[ ! -f $filename ]]; then
        wget $1
    fi

    # Verify file signature
    echo "$2 $filename" | sha256sum -c --status

    if [ $? != 0 ]; then
        echo "$(basename $0): Signature for $filename don't match."
        exit 1
    fi
}

By using something similar, the entire behavior in one function, don't you think we can add this to gitian-build.sh to keep a single file ?

P.S.: If a file is already available but do not match sha256sum, I avoid overwriting the file like it was in your code. Design choice, avoid re-downloading.

[AbcSxyZ]

For now, could we assume use docker as the default (and lonely) supported method to perform gitian ?

No.

So the script should be able to run on docker, lxc and kvm ? The suggested script is entirely removing kvm & lxc reference.

[patricklodder]

So the script should be able to run on docker, lxc and kvm ? The suggested script is entirely removing kvm & lxc reference.

Also VMWare/vagrant is not implemented. But I don't mind if in an initial iteration this script just does Docker - just have to undelete the original script.

But we cannot spot supply chain attacks on Docker (or docker images) if only we compare docker-built hashes against each other and never check against anything else. I chose to use Docker exactly because @rnicoll used to use lxc. If he would now switch to Docker, it means I'll have to switch to one of the others (most likely lxc, because that is the least obtrusive to use.)

[AbcSxyZ]

New PR built on top of this one, see #2579.

[patricklodder]

Merged with #2973! Many thanks! ❤️