1 |
On Sun, 11 Jan 2015 00:49:50 -0800 |
2 |
Zac Medico <zmedico@g.o> wrote: |
3 |
|
4 |
> Since commit f17448317166bfac42dc279b8795cd581c189582, an existing |
5 |
> symlink in /etc/config-archive could trigger a fatal "File exists" |
6 |
> error. Handle this by removing the destination file if it exists. This |
7 |
> was not necessary when dispatch-conf only supported regular files, |
8 |
> since shutil.copy2 would simply overwrite the regular destination |
9 |
> file. |
10 |
> |
11 |
> Fixes: f17448317166 ("dispatch-conf: symlink support for bug #485598") |
12 |
> X-Gentoo-Bug: 535850 |
13 |
> X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=535850 |
14 |
> --- |
15 |
> pym/portage/dispatch_conf.py | 24 ++++++++++++++++++++++++ |
16 |
> 1 file changed, 24 insertions(+) |
17 |
> |
18 |
> diff --git a/pym/portage/dispatch_conf.py |
19 |
> b/pym/portage/dispatch_conf.py index b27e68b..7d55182 100644 |
20 |
> --- a/pym/portage/dispatch_conf.py |
21 |
> +++ b/pym/portage/dispatch_conf.py |
22 |
> @@ -179,6 +179,12 @@ def rcs_archive(archive, curconf, newconf, |
23 |
> mrgconf): if curconf_st is not None and \ |
24 |
> (stat.S_ISREG(curconf_st.st_mode) or |
25 |
> stat.S_ISLNK(curconf_st.st_mode)): |
26 |
> + # Remove destination file in order to ensure that |
27 |
> the following |
28 |
> + # symlink or copy2 call won't fail (see bug #535850). |
29 |
> + try: |
30 |
> + os.unlink(archive) |
31 |
> + except OSError: |
32 |
> + pass |
33 |
> try: |
34 |
> if stat.S_ISLNK(curconf_st.st_mode): |
35 |
> os.symlink(os.readlink(curconf), |
36 |
> archive) @@ -208,6 +214,12 @@ def rcs_archive(archive, curconf, |
37 |
> newconf, mrgconf): if has_branch: |
38 |
> os.rename(archive, archive + '.dist') |
39 |
> |
40 |
> + # Remove destination file in order to ensure that |
41 |
> the following |
42 |
> + # symlink or copy2 call won't fail (see bug #535850). |
43 |
> + try: |
44 |
> + os.unlink(archive) |
45 |
> + except OSError: |
46 |
> + pass |
47 |
> try: |
48 |
> if stat.S_ISLNK(mystat.st_mode): |
49 |
> os.symlink(os.readlink(newconf), |
50 |
> archive) @@ -264,6 +276,12 @@ def file_archive(archive, curconf, |
51 |
> newconf, mrgconf): if curconf_st is not None and \ |
52 |
> (stat.S_ISREG(curconf_st.st_mode) or |
53 |
> stat.S_ISLNK(curconf_st.st_mode)): |
54 |
> + # Remove destination file in order to ensure that |
55 |
> the following |
56 |
> + # symlink or copy2 call won't fail (see bug #535850). |
57 |
> + try: |
58 |
> + os.unlink(archive) |
59 |
> + except OSError: |
60 |
> + pass |
61 |
> try: |
62 |
> if stat.S_ISLNK(curconf_st.st_mode): |
63 |
> os.symlink(os.readlink(curconf), |
64 |
> archive) @@ -285,6 +303,12 @@ def file_archive(archive, curconf, |
65 |
> newconf, mrgconf): stat.S_ISLNK(mystat.st_mode)): |
66 |
> # Save off new config file in the archive dir |
67 |
> with .dist.new suffix newconf_archive = archive + '.dist.new' |
68 |
> + # Remove destination file in order to ensure that |
69 |
> the following |
70 |
> + # symlink or copy2 call won't fail (see bug #535850). |
71 |
> + try: |
72 |
> + os.unlink(newconf_archive) |
73 |
> + except OSError: |
74 |
> + pass |
75 |
> try: |
76 |
> if stat.S_ISLNK(mystat.st_mode): |
77 |
> os.symlink(os.readlink(newconf), |
78 |
> newconf_archive) |
79 |
|
80 |
|
81 |
That makes duplicated code in 4 places :/ I know it's only 4 lines of |
82 |
actual code plus 2 more for the same repeated comments. |
83 |
|
84 |
BUT... |
85 |
|
86 |
This diff doens't show if it's all in the same class. |
87 |
|
88 |
If possible I'd prefer a small breakout function to do the unlink. |
89 |
Decorate it with @staticmethod even. If it's named right, you won't |
90 |
even need the comments repeated throughout the code. Just the |
91 |
docstring with that comment info. |
92 |
|
93 |
-- |
94 |
Brian Dolbec <dolsen> |