On Wed, Nov 16, 2016 at 4:56 PM, Nir Soffer <[email protected]> wrote:
> On Wed, Nov 16, 2016 at 9:35 AM, Yaniv Bronheim <[email protected]> > wrote: > > Hi > > > > After merging > > https://gerrit.ovirt.org/#/q/status:open+project:vdsm+ > branch:master+topic:cross-imports > > This is not related to this patch, we had to keep correct spec, > automation and dockerfile > before this patch. This patch keeps us honest, preventing wrong code > from sneaking in. > yes.. but now we see how annoying it is :) > On the first run, it found that python-requests was missing (fixed now) > and that vdsm.rpc modules are doing wrong import from /usr/share/vdsm. > These imports work by accident. This was not discovered in the review > of the patch moving rpc to lib/vdsm. > > This patch is fixing the old crossImports check that was totally broken, > checking imports is not a new concept, it simply works now. > > > we need now for any new requirement to add a line in: > > check-patch.packages.el7 > > check-patch.packages.fc24 > > check-merged.packages.el7 > > check-merged.packages.fc24 > > You forgot build-artifacts.packages.* - total of 6 files to update in > automation/ > > I suggested David Caro in the past to support reading multiple packages > files, > so you can have a check-patch.packages file, *and* check-patch.packages.el7 > and the ci will merge the list of packages, so you don't have to duplicate > the > packages 6 times in every project. > > they can use the spec as well with builddep. don't see any problem with that. > vdsm.spec.in > > Dockerfile.centos > > Dockerfile.fedora > > > > seems like we can add it once to the spec with section for fedora and > centos > > and the rest of the places will use yum-builddep. sounds more reasonable > to > > me and probably the right way to work with rpms dependencies. no? > > The issue is we have different kind of requirements: > - runtime packages > - runtime packages needed during tests (smaller set, since not all > code is tested) > - build packages (needed only for building rpms) > - check-merged packages (lago and friends?) > > with you check we require everything we import even if we don't have test for that. so basically now all of the above requires the same list > So we can use the spec as the source, and generate all the other files > during make, but this means the spec and all the *packages files will > why generating? what's wrong with rpm commands to install deps? include all the packages for the worst case, making the build even slower. > can you give an example of such package that we don't need in check-patch and built-artifacts but we need in runtime? we should test all :) > > But note that the dockerfiles must be correct regardless where you build > vdsm - you have to get different list of packages for fedora and centos > you can do that in the spec as well so you can build the docker image on any system. I'm not sure keeping > stuff in the spec will make it easy to extract for crating other files, > keeping > the files in a json / yaml file and generating the spec during configure > may > be easier. > > I think it worth the effort if we avoid updating 9 files when adding > requirements. > > Nir > -- *Yaniv Bronhaim.*
_______________________________________________ Devel mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/devel
