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.

Sign up for GitHub

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

Jump to bottom

Debian package structure and pull-upstream-changes process #5

Open
danpovey opened this issue May 16, 2018 · 11 comments
Open

Debian package structure and pull-upstream-changes process #5

danpovey opened this issue May 16, 2018 · 11 comments

Comments

@danpovey
Copy link
Member

https://salsa.debian.org/hpc-team/gridengine
@0xaf1f, I'd like to understand how that repo relates to this repo and how you pulled in changes.
In that repo, you seem to have the stuff from this repo, but also a debian/ subdirectory that contains a copy of sources/. Is that just a copy of the top-level sources/?
@danpovey
Copy link
Member Author

Oh, and just for reference: here are some notes I made on how I was able to build from the sources distributed by debian. I got the source (IIRC, apt-get source gridengine)
installed a dependency: apt-get install libpam0g-dev,

cd'd to
gridengine-8.1.9+dfsg

made changes to the following two files to account for changes in libssl; however, these changes seem to already be reflected in the upstream master (i.e. this repo), so I won't repeat them here:

source/libs/comm/cl_ssl_framework.c
source/utilbin/sge_passwd.c

and then made the following change to source/daemons/shepherd/shepherd.c which was to fix the bug with seteuid() failing:

cat ~/patches/sge.my_diff 
302,305c302,306
<    /* reset egid and euid to the stored values */
<    if (sge_seteuid(old_euid) != 0) {
<       shepherd_trace("Cannot reset euid %s due to %s", owner, strerror(errno));
<       SGE_CLOSE(fd);
---
> 
>    /* set effective user-id to root again, because only root is allowed to change
>     * the euid to any other than the current user-id.   */
>    if (sge_seteuid(SGE_SUPERUSER_UID) != 0) {
>       shepherd_trace("Cannot become root due to %s", strerror(errno));
307a309,310
>    
>    /* reset egid and euid to the stored values (e.g. those of the sgeadmin user) */
309a313,317
>       SGE_CLOSE(fd);
>       return -1;
>    }
>    if (sge_seteuid(old_euid) != 0) {
>       shepherd_trace("Cannot reset euid %s due to %s", owner, strerror(errno));

Then I built by doing (from source/):

sh scripts/bootstrap.sh
./aimk -spool-classic

This just builds, it doesn't install (I manually copied the sge_shepherd program to where it belongs, to fix our problem). @0xaf1f, do you use the -spool-classic option when installing for Debian, or it still supports berkeleydb? I seem to remember there were hard-looking compilation errors to solve without the '-spool-classic' option.

@0xaf1f
Copy link
Contributor

0xaf1f commented May 16, 2018

I'd like to understand how that repo relates to this repo and how you pulled in changes.

It's mostly an import of the release tarball. Only the debian directory is what I directly work on.

In that repo, you seem to have the stuff from this repo, but also a debian/ subdirectory that contains a copy of sources/. Is that just a copy of the top-level sources/?

debian/source is unrelated to the top-level source directory. debian/source contains usually just one file to define the Debian package format.

do you use the -spool-classic option when installing for Debian, or it still supports berkeleydb? I seem to remember there were hard-looking compilation errors to solve without the '-spool-classic' option.

It uses berkeleydb. This is the part of the packaging that runs aimk:

https://salsa.debian.org/hpc-team/gridengine/blob/a95a253e20bcbb32dc695b464d75ad758b842ab1/debian/rules#L39

Although the Debian package currently doesn't build in Unstable right now because of https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=875404

@danpovey
Copy link
Member Author

Thanks a lot for the info! I notice the debian/patches/ subdirectory contains a number of small patches, mostly to the build system. Are those already applied in your repo?

I was investigating the ways in which your Debian repo differs from this repo.
Your Debian repo (based on the commit messages) seems to think it's up to date for upstream 8.1.9, and your 'master' branch is very close to the tag '8.1.9+dfsg'. However, when I compare the code with our current upstream code for 8.1.9, I see differences.

For instance, in your
source/daemons/shepherd/builtin_starter.c
there is some old code referencing 'pre_args_ptr' that was removed in the upstream.
(it was removed in commit 9a9fe65 in this repo).

So I'm wondering whether your 8.1.9 is maybe not fully up-to-date -- perhaps you imported something that was later changed?

@0xaf1f
Copy link
Contributor

0xaf1f commented May 16, 2018

I notice the debian/patches/ subdirectory contains a number of small patches, mostly to the build system. Are those already applied in your repo?

No-- everything outside of the debian/ directory is unmodified from the release. During the package build process, the patches in debian/patches are applied and then the build is started.

I was investigating the ways in which your Debian repo differs from this repo.
Your Debian repo (based on the commit messages) seems to think it's up to date for upstream 8.1.9, and your 'master' branch is very close to the tag '8.1.9+dfsg'. However, when I compare the code with our current upstream code for 8.1.9, I see differences.

That's odd. I took directly from the release tarball and only removed a few PDFs without source (hence the +dfsg suffix), so maybe the tag was off. The orig.tar.gz tarball from Debian (http://http.debian.net/debian/pool/main/g/gridengine/gridengine_8.1.9+dfsg.orig.tar.gz) is the pristine upstream source minus the files excluded (and those files are named at the top debian/copyright in the source package).

The original place they were downloadable from is gone now, but it seems that Dave had put them up on sourceforge later on: https://sourceforge.net/projects/gridengine/files/SGE/releases/ . This release tarball for 8.1.9: https://sourceforge.net/projects/gridengine/files/SGE/releases/8.1.9/sge-8.1.9.tar.gz/download is probably it.

@danpovey
Copy link
Member Author

After looking into it more, it looks like the differences (which are fairly small) can't be explained by the 8.1.9 tag being off by a few commits. It looks like some changes from years ago didn't make it into your Debian repo, possibly due to errors in merging.

Below is an example file that differs slightly (diff from current debian master -> upstream 8.1.9):

a08:sge_debian: diff source/daemons/execd/msg_execd.h ~/sge/source/daemons/execd/msg_execd.h

210c210
< #define MSG_SYSTEM_CANTMAKETMPDIR     _MESSAGE(29122, _("can't make tmpdir"))
---
> #define MSG_SYSTEM_CANTMAKETMPDIR_S   _MESSAGE(29122, _("can't make tmpdir: "SFN))
212c212
< #define MSG_SYSTEM_CANTOPENTMPDIR_S   _MESSAGE(29124, _("can't open tmpdir "SFN))
---
> #define MSG_SYSTEM_CANTOPENTMPDIR_SS  _MESSAGE(29124, _("can't open tmpdir "SFN": "SFN))

And one of those lines, from 'git blame' in your repo, is:
33b4b66c5b (Laszlo Kajan 2013-01-30 10:42:59 +0100 212) #define MSG_SYSTEM_CANTOPENTMPDIR_S _MESSAGE(29124, _("can't open tmpdir "SFN))
while in the upstream, it's:
e4b69a70dc (Dave Love 2012-11-01 22:17:00 +0000 212) #define MSG_SYSTEM_CANTOPENTMPDIR_SS _MESSAGE(29124, _("can't open tmpdir "SFN": "SFN))

That commit by Laszlo Kajan was a merge commit (the merge of the upstream 8.1.3).
The master changed MSG_SYSTEM_CANTOPENTMPDIR_S to MSG_SYSTEM_CANTOPENTMPDIR_SS from version 8.1.3 to 8.1.4, and it looks like you guys didn't get that change.

@0xaf1f
Copy link
Contributor

0xaf1f commented May 17, 2018

I don't think it's useful to examine the git history of the upstream source in the Debian repository since it's just a series of imports from a tarball, at least since I started maintaining it. So regardless of what happened between 8.1.3 and 8.1.4, whatever exists in the 8.1.9 release tarball should be in our packages because everything got replaced by that.

However, I recently came across another repository of Dave's, which is a linear release repository: https://gitlab.com/loveshack/sge-release. I think this is what @bodgerer was talking about. At that tag for 8.1.9, line 212 of source/daemons/execd/msg_execd.h has

#define MSG_SYSTEM_CANTOPENTMPDIR_S   _MESSAGE(29124, _("can't open tmpdir "SFN))

@0xaf1f
Copy link
Contributor

0xaf1f commented May 17, 2018 •
edited
Loading

by the way, there are repos for other SGE things too: https://gitlab.com/users/loveshack/projects

@0xaf1f
Copy link
Contributor

0xaf1f commented May 17, 2018

At that tag for 8.1.9, line 212 of source/daemons/execd/msg_execd.h has

#define MSG_SYSTEM_CANTOPENTMPDIR_S _MESSAGE(29124, _("can't open tmpdir "SFN))

Here's the exact link in gitlab: https://gitlab.com/loveshack/sge-release/blob/819/source/daemons/execd/msg_execd.h#L212

@danpovey
Copy link
Member Author

danpovey commented May 17, 2018 via email

@Kunzol
Copy link
Contributor

Kunzol commented May 18, 2018

As far as I remember only Version 8.1.9 can be used for Debian Stretch because of OpenSSL version.

See Ticket 1572

and Ticket 1618

I am not sure, if it is a good idea to change the current code to make it build the packages for official Debian repos. It has some advantages to have all installed in one directory tree, e.g. distribution via network filesystem. At least there should be a configuration setting to have a choice.

@danpovey
Copy link
Member Author

danpovey commented May 19, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@Kunzol @danpovey @0xaf1f