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

os.chflags() and os.lchflags() are not built when they should be be #52992

Closed
ngie-eign mannequin opened this issue May 18, 2010 · 24 comments
Closed

os.chflags() and os.lchflags() are not built when they should be be #52992

ngie-eign mannequin opened this issue May 18, 2010 · 24 comments
Assignees
Labels
build The build process and cross-build

Comments

@ngie-eign
Copy link
Mannequin

ngie-eign mannequin commented May 18, 2010

BPO 8746
Nosy @birkenfeld, @doko42, @gpshead, @ronaldoussoren, @avassalotti, @ned-deily, @merwok, @ngie-eign
Files
  • config.log: autoconf failure log
  • issue8746-py3k.patch: patch for py3k
  • issue8746-31.patch: patch for release31-maint
  • issue8746-27.patch: patch for release27-maint
  • issue8746-trunk.patch: patch for trunk with flags defined
  • issue8746-py3k.patch: patch for py3k with flags defined
  • issue8746-test_posix.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/ned-deily'
    closed_at = <Date 2011-06-29.00:18:58.458>
    created_at = <Date 2010-05-18.09:05:18.346>
    labels = ['build']
    title = 'os.chflags() and os.lchflags() are not built when they should be be'
    updated_at = <Date 2011-07-26.21:00:20.682>
    user = 'https://github.com/ngie-eign'

    bugs.python.org fields:

    activity = <Date 2011-07-26.21:00:20.682>
    actor = 'python-dev'
    assignee = 'ned.deily'
    closed = True
    closed_date = <Date 2011-06-29.00:18:58.458>
    closer = 'ned.deily'
    components = ['Build']
    creation = <Date 2010-05-18.09:05:18.346>
    creator = 'ngie'
    dependencies = []
    files = ['17414', '18837', '18838', '18839', '18848', '18849', '22337']
    hgrepos = []
    issue_num = 8746
    keywords = ['patch']
    message_count = 24.0
    messages = ['105957', '106130', '115452', '115483', '116053', '116077', '116078', '116107', '116108', '116152', '116161', '116205', '116512', '118323', '121116', '125551', '126443', '126445', '126446', '130876', '138217', '139345', '139346', '141181']
    nosy_count = 11.0
    nosy_names = ['georg.brandl', 'doko', 'gregory.p.smith', 'ronaldoussoren', 'alexandre.vassalotti', 'ned.deily', 'eric.araujo', 'ngie', 'Nick.Dowell', 'Douglas.Leeder', 'python-dev']
    pr_nums = []
    priority = 'critical'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue8746'
    versions = ['Python 2.7', 'Python 3.2', 'Python 3.3']

    @ngie-eign
    Copy link
    Mannequin Author

    ngie-eign mannequin commented May 18, 2010

    os.chflags isn't `available' even though chflags support is available on the system:

    >>> filter(lambda x: x.startswith('chflags'), dir(os))
    []
    >>> platform.uname()
    ('FreeBSD', 'bayonetta.local', '9.0-CURRENT', 'FreeBSD 9.0-CURRENT #0 r206173M: Mon Apr 26 22:45:06 PDT 2010     root@bayonetta.local:/usr/obj/usr/src/sys/BAYONETTA.ata', 'amd64', 'amd64')
    >>> filter(lambda x: x.startswith('chflags'), dir(os))
    []
    >>> sys.version
    '2.6.5 (r265:79063, May 16 2010, 23:37:42) \n[GCC 4.2.1 20070719  [FreeBSD]]'

    I'm looking into why this is not properly detected via configure (here's the snippet though):

    configure:17257: checking for chflags
    configure:17288: cc -o conftest -O2 -pipe -fno-strict-aliasing -pipe -O2 -march=nocona -D__wchar_t=wchar_t -DTHREAD_STACK_SIZE=0x100000 -pthread conftest.c >&5
    conftest.c:173: error: expected identifier or '(' before '[' token
    In file included from /usr/include/sys/types.h:33,
    from /usr/include/sys/timespec.h:37,
    from /usr/include/sys/stat.h:42,
    from conftest.c:174:
    /usr/include/machine/types.h:75: error: expected '=', ',', ';', 'asm' or '__attribute
    ' before '__int_least8_t'
    In file included from /usr/include/sys/time.h:37,
    from /usr/include/sys/stat.h:99,
    from conftest.c:174:
    /usr/include/sys/types.h:64: error: expected '=', ',', ';', 'asm' or '__attribute
    _' before 'int8_t'
    conftest.c:182: error: expected identifier or '(' before ']' token
    configure:17291: $? = 1

    Compiling the standalone test works:

    $ cat ~/test_chflags.c 
    #include <sys/stat.h>
    #include <unistd.h>
    int main(int argc, char*argv[])
    {
      if(chflags(argv[0], 0) != 0)
        return 1;
      return 0;
    }
    $ gcc -o ~/test_chflags ~/test_chflags.c
    $ echo $?
    0

    Also, another sidenote: nowhere is *chflags(2) considered a POSIX feature (I doublechecked opengroup.org and Google). It is strictly a _Unix_ feature. I say this because the POSIX functionality tester explicitly looks for this `POSIX' compatible feature.

    @ngie-eign ngie-eign mannequin added the build The build process and cross-build label May 18, 2010
    @ngie-eign
    Copy link
    Mannequin Author

    ngie-eign mannequin commented May 20, 2010

    .

    @NickDowell
    Copy link
    Mannequin

    NickDowell mannequin commented Sep 3, 2010

    We've just noticed this same problem with Python 3.1.2 on Mac OS X.

    The problem is that the program source in the configure script has extraneous '[' and ']' characters in it, causing compilation to fail.
    Excerpt of configure from line 17357:
      cat >>conftest.$ac_ext <<_ACEOF
      /* end confdefs.h.  */
      [
      #include <sys/stat.h>
      #include <unistd.h>
      int main(int argc, char*argv[])
      {
        if(chflags(argv[0], 0) != 0)
          return 1;
        return 0;
      }
      ]
      _ACEOF

    These extra '[' and ']' characters were added to configure.in in revision 74038:
    http://svn.python.org/view/python/trunk/configure.in?r1=74033&r2=74038

    I have locally modified my configure.in and configure scripts to back out that change, and found it resolves the issue.

    @ned-deily
    Copy link
    Member

    Nick, can you provide a unit test and a patch file for the issue against the currently maintained versions? Adding Alexandre to comment on why the configure change was made.

    @ned-deily ned-deily added the build The build process and cross-build label Sep 3, 2010
    @ned-deily
    Copy link
    Member

    Thanks Garrett for reporting the problem and thank Nick for the patches. It turns out the problem is more involved though the solution is similar. I've spent some time figuring out what went wrong here and documenting it in this issue so that no one else has to. Ugh.

    The relevant checkin events:

    (1) (2009-07-16) r74038 changed the autoconf tests for chflags and lchflags to be m4a double-quoted (from '[' to '[[') to "ensure they don't get mangled". The test continues to use the AC_TRY_RUN macro and is working correctly.

    (2) (2009-11-02) r76050 checked in a modified version of a patch from Gentoo to help support cross-compiling. The original patch was against a pre-r74038 version; it wrapped the AC_TRY_RUN macro in an AC_CACHE_CHECK and a layer of quotes. In updating the patch for the then-current trunk (2.7), the significance of the r74038 change was apparently overlooked, resulting in an extra and faulty level of [...] quotes which end up in the configure test C source. The test now gets compile errors on all Unix-y platforms but it is only significant on those that do support chflags and/or lchflags, i.e. BSD ones and OS X.

    (3) (2010-01-30) The buggy test from trunk is merged into py3k (3.2) r77862, 3.1 r77863, and 2.6 r77864.

    (4) (2010-05-08) As part of bpo-8510 updates to configure.in to work with autoconf 2.65, r80478 for py3k (3.2) and r80969 for trunk (2.7) converted the AC_TRY_RUN macro calls to ones using AC_RUN_IFELSE and AC_LANG_SOURCE. An extra level of quoting is added to preserve the existing, albeit buggy syntax. The net effect is that the configure test C code generated is the same and still always fails with a compile error.

    While I have not tested this on any BSD-like systems other than OS X, I believe this means that os.chflags and/or os.lchflags have been missing-in-action from builds on OSes where they should be supported for the following releases (i.e since 2009-11-02):

    2.6.5
    2.6.6
    2.7
    3.1.2
    3.2a2

    A side note for OS X: Apple first introduced lchflags() support in OS X 10.5. Since the python.org OS X installer pythons are traditionally built to an 10.3/10.4 ABI, none of the OS X installers for the above Python releases would have had os.lchflags anyway (only os.chflags), except for the new 2.7 32-bit/64-bit installer if the 2.7 test weren't otherwise broken.

    The solution is to fix the buggy configure.in macros along the lines suggested by Nick. I have created and tested three new patches to do this:

    bpo-8746-py3k.patch    for py3k (3.2a2+)
    bpo-8746-27.patch      for release-27-maint (2.7+)
    bpo-8746-31.patch      for release-31-maint (3.1.2+)
    

    A variant of the 3.1 patch should apply to release-26-maint (2.6.6+) however 2.6 is now in security-fix-only mode.

    In each case, the patch only modifies configure.in. It is necessary to run autoreconf after applying and before checkin. A simple test of the fix is to do ./configure before and after:

        $ rm -f config.log config.status
        $ ./configure
        $ grep "expected identifier or '(' before " config.log
        conftest.c:191: error: expected identifier or '(' before '[' token
        conftest.c:200: error: expected identifier or '(' before ']' token
        conftest.c:191: error: expected identifier or '(' before '[' token
        conftest.c:200: error: expected identifier or '(' before ']' token
        $ patch <issue8746-[...].patch
        $ autoreconf
        $ rm -f config.log config.status
        $ ./configure
        $ grep "expected identifier or '(' before " config.log
        $

    Note that I have not attempted any cross-compiling testing either with or without the patches.

    @ned-deily ned-deily changed the title *chflags detection broken on FreeBSD 9-CURRENT os.chflags() and os.lchflags() are not built when they should be be Sep 10, 2010
    @ngie-eign
    Copy link
    Mannequin Author

    ngie-eign mannequin commented Sep 11, 2010

    That definitely fixes detection for FreeBSD CURRENT with 2.7 and py3k for me.

    I'm looking into providing some unit-tests, but the problem is that whether or not chflags functions on the underlying filesystem is problematic. For instance, it won't function on msdosfs (Linux calls it vfat), it won't function on ext[23] (AFAIK), and it didn't function on ZFS (until recently? I'm not sure whether or not the latest patches for ZFS enhance the filesystem to support this functionality). So unfortunately adding tests for this feature (while nice) would potentially be error prone.

    Something I tried was:

    >>> UF_IMMUTABLE = 2
    >>> import tempfile
    >>> f = tempfile.mkstemp()[1]
    >>> os.getuid()
    1000
    >>> os.chflags('/tmp', UF_IMMUTABLE)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    OSError: [Errno 1] Operation not permitted: '/tmp'
    >>> os.chflags(f, UF_IMMUTABLE)
    >>> fd = open(f, 'w')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    IOError: [Errno 1] Operation not permitted: '/tmp/tmpi_hwY7'
    >>> fd = open(f, 'r')
    >>> fd = open(f, 'r')
    >>> fd.read()
    ''
    >>> os.chflags(f, 0)
    >>> fd = open(f, 'w')
    >>> fd.write('foo')
    >>> fd.close()
    >>> fd = open(f, 'r')
    >>> fd.read()
    'foo'
    >>> fd.close()

    Also, the flags are missing that are described in the manpage. I'll provide a patch for those.

    Otherwise, it looks like everything's functioning as expected for basic end-to-end tests with chflags(2).

    Thanks!

    @ned-deily
    Copy link
    Member

    Additional tests would be great. It is probably reasonable to make some simplifying assumptions about which file systems the test directory would be run on. I'm guessing other similar file system function tests are not bulletproof on all potential file system types on any modern OS.

    @ngie-eign
    Copy link
    Mannequin Author

    ngie-eign mannequin commented Sep 11, 2010

    I'll add new tests in the next submit for the bug, but here's the code to add the relevant symbols for common to *BSD and OSX, and the Snow Leopard+ and FreeBSD specific chflags of importance for python 2.7 and py3k.

    @ngie-eign
    Copy link
    Mannequin Author

    ngie-eign mannequin commented Sep 11, 2010

    Do you prefer exhaustive tests, or just smoke tests? Honestly IMO, the OS should come prepackaged with tests to ensure that things function according to the requirements set forth in the manpage, so I would prefer the latter because the os methods in the posixmodule are nothing more than thin wrappers above the actual OS syscalls.

    @ned-deily
    Copy link
    Member

    Actually the flags do already exist: note "(as defined in the stat module)" (http://docs.python.org/py3k/library/os.html#os.chflags). Other than the new UF_HIDDEN, it looks like they are all hardwired there in Lib/stat.py. There is something to be said for the approach in your patch in that it guarantees that the flags match the build OS definitions; the downside is some lack of portability and cross-testing. Considering that there are other stat flags in there and have been for a long time, I'd be inclined to not change things other than throwing in UF_HIDDEN.

    Regarding testing, perhaps the single most important thing would be to add a test that os.chflags is in fact present on systems where it is expected (to catch any future build problems like the one in this issue). As you say, these are relatively transparent wrappers and we have to accept the OS's implementation. Testing for the presence of os.lchflags is a bit trickier in that on OS X it should only be present if Python was built with a MACOSX_DEPLOYMENT_TARGET of 10.5 or higher.

    @NickDowell
    Copy link
    Mannequin

    NickDowell mannequin commented Sep 12, 2010

    Why should it only be available on OS X if built with MACOSX_DEPLOYMENT_TARGET of 10.5 or higher?
    chflags() should be available in earlier versions of the OS:
    http://developer.apple.com/library/mac/#documentation/Darwin/Reference/ManPages/10.3/man2/chflags.2.html

    @ned-deily
    Copy link
    Member

    Nick: "lchflags". lchflags is only available in 10.5 or higher, unlike chflags which has been around for much longer.

    @ngie-eign
    Copy link
    Mannequin Author

    ngie-eign mannequin commented Sep 16, 2010

    *NOUNLINK was not implemented by OSX, so there's actually a bug with the compile tests if you take that into consideration. And again, only FreeBSD defines *NOUNLINK. The other BSDs, not so much.

    As far as the reason why I implemented the flags, the documentation that I was looking at didn't implement those flags. I looked at the latest documentation and it appears to be correct though. I will implement tests for *UNLINK and *HIDDEN (FWIW the tests I wrote are ok for cross-compilation because they just test for the existence of the values -- they don't test to ensure that the functionality is correct, like some of the logic in configure was recently `fixed' to do).

    @birkenfeld
    Copy link
    Member

    This can be solved after 3.2a3.

    @birkenfeld
    Copy link
    Member

    Same for a4.

    @ned-deily
    Copy link
    Member

    Ping - Garrett, any progress on updating the tests? With dispatch, this could still make it into 3.2.

    @ngie-eign
    Copy link
    Mannequin Author

    ngie-eign mannequin commented Jan 18, 2011

    Sorry -- got off-track for a while.

    I assume these should go into Lib/test/test_os.py ?

    @ned-deily
    Copy link
    Member

    There is an existing test_lchflags in Lib/test/test_posix.py. Additional test cases should go there.

    @ngie-eign
    Copy link
    Mannequin Author

    ngie-eign mannequin commented Jan 18, 2011

    On Mon, Jan 17, 2011 at 5:11 PM, Ned Deily <report@bugs.python.org> wrote:

    Ned Deily <nad@acm.org> added the comment:

    There is an existing test_lchflags in Lib/test/test_posix.py.  Additional test cases should go there.

    Ok, but again this isn't POSIX functionality -- it's a BSD functional piece.

    Another thing that's nasty that I've discovered is that the
    

    function prototype isn't the same across the board. After things are
    consolidated in FreeBSD I'll talk to the NetBSD and OpenBSD folks.
    Thanks!
    -Garrett

    @ronaldoussoren
    Copy link
    Contributor

    Tests for the posix module should go in test_posix. The name 'posix' is used very loosely in python, it is basicly used as an alias for 'unixy'.

    I'm not too happy about replicating the definition of the flags, as Ned noted the flags are also defined in Lib/stat.py. I'd keep them out of posixmodule as the current definitions in Lib/stat.py should be valid on all platforms that currently support these flags.

    W.r.t testing: there should be some simple smoke tests, basicly just enough to ensure that the function is available and that it links up to the right system call. There is no need to perform un exhaustive test of the system call, that should be done by the os vendor.

    @ngie-eign
    Copy link
    Mannequin Author

    ngie-eign mannequin commented Jun 12, 2011

    I apologize for taking so long with this. The attached patch is for test_posix against trunk (I shuffled around some code and extended some things to improve PosixTester.tearDown). Let me know if I need to wash, rinse, repeat for py3k, etc.

    Thanks!

    # Standard ports copy of python 2.7 without the *chflags configure fix.

    $ python2.7 Lib/test/test_posix.py 
    testNoArgFunctions (__main__.PosixTester) ... ok
    test_access (__main__.PosixTester) ... ok
    test_chdir (__main__.PosixTester) ... ok
    test_chflags (__main__.PosixTester) ... skipped 'test needs os.chflags()'
    test_chown (__main__.PosixTester) ... ok
    test_confstr (__main__.PosixTester) ... ok
    test_dup (__main__.PosixTester) ... ok
    test_dup2 (__main__.PosixTester) ... ok
    test_fchown (__main__.PosixTester) ... ok
    test_fdopen (__main__.PosixTester) ... ok
    test_fstat (__main__.PosixTester) ... ok
    test_fstatvfs (__main__.PosixTester) ... ok
    test_ftruncate (__main__.PosixTester) ... ok
    test_getcwd_long_pathnames (__main__.PosixTester) ... ok
    test_getresgid (__main__.PosixTester) ... ok
    test_getresuid (__main__.PosixTester) ... ok
    test_initgroups (__main__.PosixTester) ... ok
    test_lchflags_regular_file (__main__.PosixTester) ... skipped 'test needs os.lchflags()'
    test_lchflags_symlink (__main__.PosixTester) ... skipped 'test needs os.lchflags()'
    test_lchown (__main__.PosixTester) ... ok
    test_lsdir (__main__.PosixTester) ... ok
    test_osexlock (__main__.PosixTester) ... ok
    test_osshlock (__main__.PosixTester) ... ok
    test_pipe (__main__.PosixTester) ... ok
    test_setresgid (__main__.PosixTester) ... ok
    test_setresgid_exception (__main__.PosixTester) ... ok
    test_setresuid (__main__.PosixTester) ... ok
    test_setresuid_exception (__main__.PosixTester) ... ok
    test_stat (__main__.PosixTester) ... ok
    test_statvfs (__main__.PosixTester) ... ok
    test_strerror (__main__.PosixTester) ... ok
    test_tempnam (__main__.PosixTester) ... ok
    test_tmpfile (__main__.PosixTester) ... ok
    test_umask (__main__.PosixTester) ... ok
    test_utime (__main__.PosixTester) ... ok

    Ran 35 tests in 0.020s

    OK (skipped=3)

    # Hand compiled version using sources from trunk.

    $ /scratch/python/2.7/bin/python Lib/test/test_posix.py 
    testNoArgFunctions (__main__.PosixTester) ... ok
    test_access (__main__.PosixTester) ... ok
    test_chdir (__main__.PosixTester) ... ok
    test_chflags (__main__.PosixTester) ... ok
    test_chown (__main__.PosixTester) ... ok
    test_confstr (__main__.PosixTester) ... ok
    test_dup (__main__.PosixTester) ... ok
    test_dup2 (__main__.PosixTester) ... ok
    test_fchown (__main__.PosixTester) ... ok
    test_fdopen (__main__.PosixTester) ... ok
    test_fstat (__main__.PosixTester) ... ok
    test_fstatvfs (__main__.PosixTester) ... ok
    test_ftruncate (__main__.PosixTester) ... ok
    test_getcwd_long_pathnames (__main__.PosixTester) ... ok
    test_getresgid (__main__.PosixTester) ... ok
    test_getresuid (__main__.PosixTester) ... ok
    test_initgroups (__main__.PosixTester) ... ok
    test_lchflags_regular_file (__main__.PosixTester) ... ok
    test_lchflags_symlink (__main__.PosixTester) ... ok
    test_lchown (__main__.PosixTester) ... ok
    test_lsdir (__main__.PosixTester) ... ok
    test_osexlock (__main__.PosixTester) ... ok
    test_osshlock (__main__.PosixTester) ... ok
    test_pipe (__main__.PosixTester) ... ok
    test_setresgid (__main__.PosixTester) ... ok
    test_setresgid_exception (__main__.PosixTester) ... ok
    test_stat (__main__.PosixTester) ... ok
    test_statvfs (__main__.PosixTester) ... ok
    test_strerror (__main__.PosixTester) ... ok
    test_tempnam (__main__.PosixTester) ... ok
    test_tmpfile (__main__.PosixTester) ... ok
    test_umask (__main__.PosixTester) ... ok
    test_utime (__main__.PosixTester) ... ok

    Ran 35 tests in 0.009s

    OK

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 28, 2011

    New changeset abfe28e7e5cd by Ned Deily in branch '2.7':
    Issue bpo-8746: Correct faulty configure checks so that os.chflags() and
    http://hg.python.org/cpython/rev/abfe28e7e5cd

    New changeset 529e26aa4fa3 by Ned Deily in branch '3.2':
    Issue bpo-8746: Correct faulty configure checks so that os.chflags() and
    http://hg.python.org/cpython/rev/529e26aa4fa3

    New changeset 9da64c0bdc33 by Ned Deily in branch 'default':
    Issue bpo-8746: Correct faulty configure checks so that os.chflags() and
    http://hg.python.org/cpython/rev/9da64c0bdc33

    @ned-deily
    Copy link
    Member

    Thanks for the additional tests, Garrett. I've applied them (modulo a fix). I've also applied the corrections to configure which should make os.chflags() and os.lchflags() reappear again in BSD and OS X builds where supported. I've also added and documented two new OS X specific file flags: UF_HIDDEN and UF_COMPRESSED. Applied in default (for 3.3), 3.2 (3.2.1), and 2.7 (2.7.3).

    @ned-deily ned-deily removed the build The build process and cross-build label Jun 28, 2011
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 26, 2011

    New changeset ca236138f0ce by Ned Deily in branch '2.7':
    Issue bpo-8746: Use tempfile module to get tempdir and randomize the
    http://hg.python.org/cpython/rev/ca236138f0ce

    New changeset 4e4554aa1453 by Ned Deily in branch '3.2':
    Issue bpo-8746: Use tempfile module to get tempdir and randomize the
    http://hg.python.org/cpython/rev/4e4554aa1453

    New changeset 080035e58bab by Ned Deily in branch 'default':
    Issue bpo-8746: Use tempfile module to get tempdir and randomize the
    http://hg.python.org/cpython/rev/080035e58bab

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    build The build process and cross-build
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants