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

Changes to base class does not change package hash #30720

Open
3 tasks done
iarspider opened this issue May 18, 2022 · 2 comments
Open
3 tasks done

Changes to base class does not change package hash #30720

iarspider opened this issue May 18, 2022 · 2 comments

Comments

@iarspider
Copy link
Contributor

iarspider commented May 18, 2022

Steps to reproduce

$ spack create -n cmsroot -t cmake https://github.com/cms-sw/root/
$ cat var/spack/repos/builtin/packages/cmsroot/package.py
from spack.pkg.builtin.root import Root as BuiltinRoot


class Cmsroot(BuiltinRoot):
    __doc__ = BuiltinRoot.__doc__

    git      = "https://github.com/cms-sw/root.git"

    version('1.2.3', '0123456789abcdef0123456789abcdef')

    def cmake_args(self):
        args = []
        replace_args = ('builtin_ftgl', 'builtin_gl2ps', 'builtin_glew')
        for arg in super().cmake_args():
            for rarg in replace_args:
                if arg.startswith('-D' + rarg):
                    args.append(self.define(rarg, True))
                    break
            else:
                args.append(arg)

        return args
$ spack spec --format '{hash}' cmsroot
idkrysrzi72lmyiah6ne2gyswrduzhy5
$ sed -i -e "s/return options/options.append(define('FOO', 'BAR'))\n        return options"
$ spack spec --format '{hash}' cmsroot
idkrysrzi72lmyiah6ne2gyswrduzhy5

Error message

Hash should be different

Information on your system

  • Spack: 0.18.0.dev0 (c775c32)
  • Python: 3.10.4
  • Platform: linux-fedora36-skylake
  • Concretizer: clingo

General information

  • I have run spack debug report and reported the version of Spack/Python/Platform
  • I have searched the issues of this repo and believe this is not a duplicate
  • I have run the failing commands in debug mode and reported the output
@iarspider iarspider added bug triage The issue needs to be prioritized labels May 18, 2022
@iarspider
Copy link
Contributor Author

A bit of context: in CMS, we modify many recipes to match our setup (e.g. we drop documentation and static libraries to reduce the image size). In order not to copy and patch the upstream recipe every time it updates, we decided to use inheritance. However, it was discovered that if upstream package changes we may miss the changes in our build since the hash of the package will not change.

@becker33 becker33 added this to To do in Spack v0.18.0 release via automation May 18, 2022
@becker33
Copy link
Member

@tgamblin how feasible is it to fix this for the release? I think we can do the static version of this (canonicalize all modules in class hierarchy between package and PackageBase) and not worry about whether those changes would be masked out by an overriding method the inheriting class.

@tgamblin tgamblin removed this from To do in Spack v0.18.0 release May 28, 2022
@tgamblin tgamblin added this to To do in Spack v0.19.0 release via automation May 28, 2022
@tgamblin tgamblin added this to the v0.20.0 milestone Nov 7, 2022
@tgamblin tgamblin removed this from To do in Spack v0.19.0 release Nov 7, 2022
@alalazo alalazo modified the milestones: v0.20.0, v0.21.0 May 2, 2023
@alalazo alalazo removed this from the v0.21.0 milestone Jun 7, 2023
@alalazo alalazo added impact-low hashes and removed triage The issue needs to be prioritized labels Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

No branches or pull requests

4 participants