Re: [gentoo-portage-dev] [PATCH v2] Move INSTALL_MASK handling into merging
On 06/12/2016 01:43 PM, Zac Medico wrote: > On 06/12/2016 01:29 PM, Zac Medico wrote: >> On 05/22/2016 01:21 AM, Michał Górny wrote: >>> diff --git a/pym/portage/package/ebuild/config.py >>> b/pym/portage/package/ebuild/config.py >>> index fcc7ce5..9d13703 100644 >>> --- a/pym/portage/package/ebuild/config.py >>> +++ b/pym/portage/package/ebuild/config.py >>> @@ -1774,14 +1774,14 @@ class config(object): >>> _eapi_cache.clear() >>> >>> # Prepare the final value of INSTALL_MASK >>> - install_mask = self["INSTALL_MASK"].split() >>> + install_mask = self.get("INSTALL_MASK", "").split() >>> if 'nodoc' in self.features: >>> install_mask.append("/usr/share/doc") >>> if 'noinfo' in self.features: >>> install_mask.append("/usr/share/info") >>> if 'noman' in self.features: >>> install_mask.append("/usr/share/man") >>> - self["INSTALL_MASK"] = ' '.join(install_mask) >>> + self.install_mask = install_mask >>> >>> def _grab_pkg_env(self, penv, container, protected_keys=None): >>> if protected_keys is None: >>> >> >> The config.reset method should reset self.install_mask to match the >> global INSTALL_MASK setting. >> > > For the benefit of those who may not be as familiar with the config > class, here is some of the related irc discussion: > > mgorny: config.setcpv put's the config into per-package state, > and config.reset reverts it to global state > i see > so how does that affect me? ;-D > because yout set self.install_mask in config.setcpv > so you also have to handle global setting of that attribute > every attribute has global and per-package state > for example the usemask attribute is similar > initially it has global use.mask settings > hmm, so you mean i can't rely on both being combined properly > in .setcpv() ? > hmm, but wait, do i need to care about global state at all? > yes, that's what config.reset is for > it transitions back to global state > but is there any case when i need the global state? > i don't think the mask is used without packages > if that's the case, the you can set it to None whien the > config is in global mode > careful though, setcpv returns early in some cases > and you set self.install_mask at the very end of that method > so it's not guaranteed to execute > so you have end up with self.install_mask having the global > setting after a setcpv call > which is fine if there's no package.env setting for that package > mgorny: also, you should copy self.install_mask in the clone part of the config constructor -- Thanks, Zac
Re: [gentoo-portage-dev] [PATCH v2] Move INSTALL_MASK handling into merging
On 06/12/2016 01:29 PM, Zac Medico wrote: > On 05/22/2016 01:21 AM, Michał Górny wrote: >> diff --git a/pym/portage/package/ebuild/config.py >> b/pym/portage/package/ebuild/config.py >> index fcc7ce5..9d13703 100644 >> --- a/pym/portage/package/ebuild/config.py >> +++ b/pym/portage/package/ebuild/config.py >> @@ -1774,14 +1774,14 @@ class config(object): >> _eapi_cache.clear() >> >> # Prepare the final value of INSTALL_MASK >> -install_mask = self["INSTALL_MASK"].split() >> +install_mask = self.get("INSTALL_MASK", "").split() >> if 'nodoc' in self.features: >> install_mask.append("/usr/share/doc") >> if 'noinfo' in self.features: >> install_mask.append("/usr/share/info") >> if 'noman' in self.features: >> install_mask.append("/usr/share/man") >> -self["INSTALL_MASK"] = ' '.join(install_mask) >> +self.install_mask = install_mask >> >> def _grab_pkg_env(self, penv, container, protected_keys=None): >> if protected_keys is None: >> > > The config.reset method should reset self.install_mask to match the > global INSTALL_MASK setting. > For the benefit of those who may not be as familiar with the config class, here is some of the related irc discussion: mgorny: config.setcpv put's the config into per-package state, and config.reset reverts it to global state i see so how does that affect me? ;-D because yout set self.install_mask in config.setcpv so you also have to handle global setting of that attribute every attribute has global and per-package state for example the usemask attribute is similar initially it has global use.mask settings hmm, so you mean i can't rely on both being combined properly in .setcpv() ? hmm, but wait, do i need to care about global state at all? yes, that's what config.reset is for it transitions back to global state but is there any case when i need the global state? i don't think the mask is used without packages if that's the case, the you can set it to None whien the config is in global mode careful though, setcpv returns early in some cases and you set self.install_mask at the very end of that method so it's not guaranteed to execute so you have end up with self.install_mask having the global setting after a setcpv call which is fine if there's no package.env setting for that package -- Thanks, Zac
Re: [gentoo-portage-dev] [PATCH v2] Move INSTALL_MASK handling into merging
On 05/22/2016 01:21 AM, Michał Górny wrote: > diff --git a/pym/portage/package/ebuild/config.py > b/pym/portage/package/ebuild/config.py > index fcc7ce5..9d13703 100644 > --- a/pym/portage/package/ebuild/config.py > +++ b/pym/portage/package/ebuild/config.py > @@ -1774,14 +1774,14 @@ class config(object): > _eapi_cache.clear() > > # Prepare the final value of INSTALL_MASK > - install_mask = self["INSTALL_MASK"].split() > + install_mask = self.get("INSTALL_MASK", "").split() > if 'nodoc' in self.features: > install_mask.append("/usr/share/doc") > if 'noinfo' in self.features: > install_mask.append("/usr/share/info") > if 'noman' in self.features: > install_mask.append("/usr/share/man") > - self["INSTALL_MASK"] = ' '.join(install_mask) > + self.install_mask = install_mask > > def _grab_pkg_env(self, penv, container, protected_keys=None): > if protected_keys is None: > The config.reset method should reset self.install_mask to match the global INSTALL_MASK setting. -- Thanks, Zac
Re: [gentoo-portage-dev] [PATCH v2] Move INSTALL_MASK handling into merging
On 06/12/2016 03:05 AM, Michał Górny wrote: > Dnia 12 czerwca 2016 11:49:26 CEST, Zac Medico> napisał(a): >> On 06/12/2016 02:28 AM, Michał Górny wrote: >>> Dnia 12 czerwca 2016 11:10:55 CEST, Zac Medico >> napisał(a): On 05/22/2016 01:21 AM, Michał Górny wrote: > Introduce a new logic for INSTALL_MASK handling in merging code, > replacing the old code that removed matching files and directories > from imagedir in bash. The new code actually ignores matching files > on-the-fly while testing for file collisions and merging files. > The files are still written to CONTENTS, and output using "###" >> zing > to indicate being masked, yet are not actually merged to the filesystem. Since collision-protect relies on existing files in its collision >> test, install-masked files are no longer going to trigger collisions. >> Then, since the install-masked files are still written to CONTENTS, it's possible for the unmerge of one package to unmerge colliding files >> that belong to another package! There are a number of ways to solve this problem. For example, we >> could have the unmerge code ignore any files in CONTENTS that match the INSTALL_MASK value that was used at merge time. >>> >>> Hmm, thinking about this more widely (i.e. actually thinking rather >> than mimicking the old behavior), I think it would be better to >> actually use the original file set for collision-protect. This will >> make it possible to detect collisions that would otherwise be hidden >> via INSTALL_MASK. >> >> Even then, we have to carefully consider how the absence of installed >> files affects the collision test. It's safest for the unmerge code to >> be >> aware of the merge time INSTALL_MASK setting, and not try to unmerge >> the >> install-masked files. > > But then it wouldn't unmerge the newly masked files as well. I'm suggesting to save the INSTALL_MASK setting from merge time, so any new INSTALL_MASK settings since then are irrelevant. > Getting this right will require a lot of effort, and we're less likely to > screw something up if we keep it simple. You can use exactly the same pattern matching code for merge and unmerge. It's worthwhile, given the say that collision-protect relies on file existence in the collision test. -- Thanks, Zac
Re: [gentoo-portage-dev] [PATCH v2] Move INSTALL_MASK handling into merging
Dnia 12 czerwca 2016 11:49:26 CEST, Zac Mediconapisał(a): >On 06/12/2016 02:28 AM, Michał Górny wrote: >> Dnia 12 czerwca 2016 11:10:55 CEST, Zac Medico >napisał(a): >>> On 05/22/2016 01:21 AM, Michał Górny wrote: Introduce a new logic for INSTALL_MASK handling in merging code, replacing the old code that removed matching files and directories from imagedir in bash. The new code actually ignores matching files on-the-fly while testing for file collisions and merging files. The files are still written to CONTENTS, and output using "###" >zing to indicate being masked, yet are not actually merged to the >>> filesystem. >>> >>> Since collision-protect relies on existing files in its collision >test, >>> install-masked files are no longer going to trigger collisions. >Then, >>> since the install-masked files are still written to CONTENTS, it's >>> possible for the unmerge of one package to unmerge colliding files >that >>> belong to another package! >>> >>> There are a number of ways to solve this problem. For example, we >could >>> have the unmerge code ignore any files in CONTENTS that match the >>> INSTALL_MASK value that was used at merge time. >> >> Hmm, thinking about this more widely (i.e. actually thinking rather >than mimicking the old behavior), I think it would be better to >actually use the original file set for collision-protect. This will >make it possible to detect collisions that would otherwise be hidden >via INSTALL_MASK. > >Even then, we have to carefully consider how the absence of installed >files affects the collision test. It's safest for the unmerge code to >be >aware of the merge time INSTALL_MASK setting, and not try to unmerge >the >install-masked files. But then it wouldn't unmerge the newly masked files as well. Getting this right will require a lot of effort, and we're less likely to screw something up if we keep it simple. -- Best regards, Michał Górny (by phone)
Re: [gentoo-portage-dev] [PATCH v2] Move INSTALL_MASK handling into merging
On 06/12/2016 02:28 AM, Michał Górny wrote: > Dnia 12 czerwca 2016 11:10:55 CEST, Zac Medico> napisał(a): >> On 05/22/2016 01:21 AM, Michał Górny wrote: >>> Introduce a new logic for INSTALL_MASK handling in merging code, >>> replacing the old code that removed matching files and directories >>> from imagedir in bash. The new code actually ignores matching files >>> on-the-fly while testing for file collisions and merging files. >>> The files are still written to CONTENTS, and output using "###" zing >>> to indicate being masked, yet are not actually merged to the >> filesystem. >> >> Since collision-protect relies on existing files in its collision test, >> install-masked files are no longer going to trigger collisions. Then, >> since the install-masked files are still written to CONTENTS, it's >> possible for the unmerge of one package to unmerge colliding files that >> belong to another package! >> >> There are a number of ways to solve this problem. For example, we could >> have the unmerge code ignore any files in CONTENTS that match the >> INSTALL_MASK value that was used at merge time. > > Hmm, thinking about this more widely (i.e. actually thinking rather than > mimicking the old behavior), I think it would be better to actually use the > original file set for collision-protect. This will make it possible to detect > collisions that would otherwise be hidden via INSTALL_MASK. Even then, we have to carefully consider how the absence of installed files affects the collision test. It's safest for the unmerge code to be aware of the merge time INSTALL_MASK setting, and not try to unmerge the install-masked files. -- Thanks, Zac
Re: [gentoo-portage-dev] [PATCH v2] Move INSTALL_MASK handling into merging
Dnia 12 czerwca 2016 11:10:55 CEST, Zac Mediconapisał(a): >On 05/22/2016 01:21 AM, Michał Górny wrote: >> Introduce a new logic for INSTALL_MASK handling in merging code, >> replacing the old code that removed matching files and directories >> from imagedir in bash. The new code actually ignores matching files >> on-the-fly while testing for file collisions and merging files. >> The files are still written to CONTENTS, and output using "###" zing >> to indicate being masked, yet are not actually merged to the >filesystem. > >Since collision-protect relies on existing files in its collision test, >install-masked files are no longer going to trigger collisions. Then, >since the install-masked files are still written to CONTENTS, it's >possible for the unmerge of one package to unmerge colliding files that >belong to another package! > >There are a number of ways to solve this problem. For example, we could >have the unmerge code ignore any files in CONTENTS that match the >INSTALL_MASK value that was used at merge time. Hmm, thinking about this more widely (i.e. actually thinking rather than mimicking the old behavior), I think it would be better to actually use the original file set for collision-protect. This will make it possible to detect collisions that would otherwise be hidden via INSTALL_MASK. -- Best regards, Michał Górny (by phone)
Re: [gentoo-portage-dev] [PATCH v2] Move INSTALL_MASK handling into merging
On 05/22/2016 01:21 AM, Michał Górny wrote: > Introduce a new logic for INSTALL_MASK handling in merging code, > replacing the old code that removed matching files and directories > from imagedir in bash. The new code actually ignores matching files > on-the-fly while testing for file collisions and merging files. > The files are still written to CONTENTS, and output using "###" zing > to indicate being masked, yet are not actually merged to the filesystem. Since collision-protect relies on existing files in its collision test, install-masked files are no longer going to trigger collisions. Then, since the install-masked files are still written to CONTENTS, it's possible for the unmerge of one package to unmerge colliding files that belong to another package! There are a number of ways to solve this problem. For example, we could have the unmerge code ignore any files in CONTENTS that match the INSTALL_MASK value that was used at merge time. -- Thanks, Zac
Re: [gentoo-portage-dev] [PATCH v2] Move INSTALL_MASK handling into merging
On 05/22/2016 01:21 AM, Michał Górny wrote: > + def _is_install_masked(self, relative_path): > + ret = False > + for pattern in self.settings.install_mask: > + # absolute path pattern > + if pattern.startswith('/'): > + # match either exact path or one of parent dirs > + # the latter is done via matching pattern/* > + if (fnmatch.fnmatch(relative_path, pattern[1:]) > + or > fnmatch.fnmatch(relative_path, pattern[1:] + '/*')): > + ret = True > + break > + # filename > + else: > + if > fnmatch.fnmatch(os.path.basename(relative_path), pattern): > + ret = True > + break > + return ret > + This is a hot spot, so it should use a pre-compiled regular expression, using | to join the results of fnmatch.translate(pattern) calls for each pattern. -- Thanks, Zac